From 86cd139ac4ceb437ab29122c4ccbc810e2498af7 Mon Sep 17 00:00:00 2001 From: Cesar-M-Diaz Date: Wed, 19 Aug 2026 17:22:03 +0200 Subject: [PATCH 1/8] feat(skills): add ns-download-asset skill; stream fetch-asset.cjs to disk Move raw asset download off the deprecated MCP 'asset' tool (removed from the console's MCP surface; it inlined huge raw payloads and killed MCP sessions) into a dedicated ns-download-asset skill. - skill-assets/fetch-asset.cjs: replace buffered res.text() download with a streaming pipeline (Readable.fromWeb -> createWriteStream), constant memory regardless of asset size; raise total timeout 120s -> 10min for large snapshots over slow links; rename fetchAsset -> downloadAsset. - packages/core/test/unit/skills/fetch-asset.test.ts: unit tests for downloadAsset (streams bytes to disk, returns size, sends service-token + Accept headers, URL-encodes asset ID, 404 throws without file). - skills/ns-download-asset/SKILL.md: new skill (identify asset, resolve assetType/appName, download via bundled script, report path/size) with guardrails incl. never using the MCP asset tool or reading raw assets into context. - bundle.json + packages/core/bundle.json: register ns-download-asset (regenerated root manifests via plugin:root: .claude-plugin/plugin.json). - skill-assets.manifest.json: fetch-asset.cjs now synced into 6 skills. - Add MCP-asset-tool guardrail line to the 5 existing asset skills (version-skew protection against older consoles). --- .claude-plugin/plugin.json | 1 + bundle.json | 6 + packages/core/bundle.json | 6 + .../core/scripts/skill-assets.manifest.json | 1 + .../core/test/unit/skills/fetch-asset.test.ts | 65 +++ skill-assets/fetch-asset.cjs | 19 +- .../ns-advanced-memory-leak-hunter/SKILL.md | 3 +- .../fetch-asset.cjs | 19 +- skills/ns-analyze-asset/SKILL.md | 1 + skills/ns-analyze-asset/fetch-asset.cjs | 19 +- skills/ns-cpu-spike-analysis/SKILL.md | 1 + skills/ns-cpu-spike-analysis/fetch-asset.cjs | 19 +- skills/ns-download-asset/SKILL.md | 44 ++ skills/ns-download-asset/fetch-asset.cjs | 441 ++++++++++++++++++ skills/ns-generate-asset/SKILL.md | 1 + skills/ns-generate-asset/fetch-asset.cjs | 19 +- skills/ns-memory-spike-analysis/SKILL.md | 1 + .../ns-memory-spike-analysis/fetch-asset.cjs | 19 +- 18 files changed, 642 insertions(+), 43 deletions(-) create mode 100644 skills/ns-download-asset/SKILL.md create mode 100644 skills/ns-download-asset/fetch-asset.cjs diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json index bec2a76..b572a50 100644 --- a/.claude-plugin/plugin.json +++ b/.claude-plugin/plugin.json @@ -19,6 +19,7 @@ "./skills/ns-audit-dependencies", "./skills/ns-benchmark-run", "./skills/ns-cpu-spike-analysis", + "./skills/ns-download-asset", "./skills/ns-generate-asset", "./skills/ns-generate-sbom", "./skills/ns-memory-spike-analysis", diff --git a/bundle.json b/bundle.json index a84be1b..d313da8 100644 --- a/bundle.json +++ b/bundle.json @@ -51,6 +51,12 @@ "description": "Profile CPU usage of running production processes to identify bottlenecks", "requiresMcp": ["nsolid-console"] }, + { + "name": "ns-download-asset", + "path": "skills/ns-download-asset", + "description": "Download a raw N|Solid diagnostic asset (heap snapshot, CPU profile, heap profile) to local disk", + "requiresMcp": ["nsolid-console"] + }, { "name": "ns-generate-asset", "path": "skills/ns-generate-asset", diff --git a/packages/core/bundle.json b/packages/core/bundle.json index a84be1b..d313da8 100644 --- a/packages/core/bundle.json +++ b/packages/core/bundle.json @@ -51,6 +51,12 @@ "description": "Profile CPU usage of running production processes to identify bottlenecks", "requiresMcp": ["nsolid-console"] }, + { + "name": "ns-download-asset", + "path": "skills/ns-download-asset", + "description": "Download a raw N|Solid diagnostic asset (heap snapshot, CPU profile, heap profile) to local disk", + "requiresMcp": ["nsolid-console"] + }, { "name": "ns-generate-asset", "path": "skills/ns-generate-asset", diff --git a/packages/core/scripts/skill-assets.manifest.json b/packages/core/scripts/skill-assets.manifest.json index cf37e91..7916ceb 100644 --- a/packages/core/scripts/skill-assets.manifest.json +++ b/packages/core/scripts/skill-assets.manifest.json @@ -12,6 +12,7 @@ "ns-advanced-memory-leak-hunter", "ns-analyze-asset", "ns-cpu-spike-analysis", + "ns-download-asset", "ns-generate-asset", "ns-memory-spike-analysis" ] diff --git a/packages/core/test/unit/skills/fetch-asset.test.ts b/packages/core/test/unit/skills/fetch-asset.test.ts index 0a11580..adf89d1 100644 --- a/packages/core/test/unit/skills/fetch-asset.test.ts +++ b/packages/core/test/unit/skills/fetch-asset.test.ts @@ -3,6 +3,8 @@ import assert from 'node:assert/strict' import { promises as dns } from 'node:dns' import { fileURLToPath, pathToFileURL } from 'node:url' import { dirname, join } from 'node:path' +import { mkdtemp, readFile, rm, stat } from 'node:fs/promises' +import { tmpdir } from 'node:os' import type { TestContext } from 'node:test' const __dirname = dirname(fileURLToPath(import.meta.url)) @@ -31,6 +33,7 @@ async function loadFetchAsset () { isPrivateOrLocalIp: (ip: string) => boolean resolveHostnameIps: (hostname: string) => Promise validateConsoleUrl: (consoleUrl: string) => Promise + downloadAsset: (consoleUrl: string, token: string, assetId: string, destPath: string) => Promise } } @@ -247,3 +250,65 @@ describe('validateConsoleUrl', () => { ) }) }) + +describe('downloadAsset', () => { + let tempDir: string + + beforeEach(async () => { + tempDir = await mkdtemp(join(tmpdir(), 'fetch-asset-download-')) + }) + + afterEach(async () => { + await rm(tempDir, { recursive: true, force: true }) + }) + + it('streams the asset body to disk and returns the file size', async (t) => { + const payload = new Uint8Array([0, 1, 2, 3, 4, 5, 6, 7, 8, 9]) + const calls: Array<{ url: string, headers: Record }> = [] + t.mock.method(globalThis, 'fetch', (async (input: RequestInfo | URL, init?: RequestInit) => { + calls.push({ url: String(input), headers: (init?.headers ?? {}) as Record }) + return new Response(payload) + }) as typeof fetch) + + const { downloadAsset } = await loadFetchAsset() + const destPath = join(tempDir, 'heapsnapshot-test.heapsnapshot') + + const size = await downloadAsset('https://console.example.test', 'secret-token', 'asset-id-1', destPath) + + const stats = await stat(destPath) + assert.equal(size, stats.size, 'returns the on-disk file size') + assert.equal(size, payload.byteLength) + assert.deepEqual(await readFile(destPath), Buffer.from(payload), 'file bytes match the response body') + assert.equal(calls.length, 1) + assert.equal(calls[0].url, 'https://console.example.test/api/v3/asset/asset-id-1') + assert.equal(calls[0].headers['x-nsolid-service-token'], 'secret-token') + assert.equal(calls[0].headers.Accept, 'application/json') + }) + + it('URL-encodes the asset ID in the request path', async (t) => { + const urls: string[] = [] + t.mock.method(globalThis, 'fetch', (async (input: RequestInfo | URL) => { + urls.push(String(input)) + return new Response(new Uint8Array([1, 2, 3])) + }) as typeof fetch) + + const { downloadAsset } = await loadFetchAsset() + await downloadAsset('https://console.example.test', 'token', 'id/with space', join(tempDir, 'a.heapsnapshot')) + + assert.equal(urls[0], 'https://console.example.test/api/v3/asset/id%2Fwith%20space') + }) + + it('throws on non-ok responses without creating a file', async (t) => { + t.mock.method(globalThis, 'fetch', (async () => { + return new Response('not found', { status: 404, statusText: 'Not Found' }) + }) as typeof fetch) + + const { downloadAsset } = await loadFetchAsset() + const destPath = join(tempDir, 'missing.heapsnapshot') + await assert.rejects( + () => downloadAsset('https://console.example.test', 'token', 'missing-id', destPath), + /404 Not Found for asset missing-id/ + ) + await assert.rejects(() => stat(destPath), /ENOENT/) + }) +}) diff --git a/skill-assets/fetch-asset.cjs b/skill-assets/fetch-asset.cjs index b41c216..932dfe9 100644 --- a/skill-assets/fetch-asset.cjs +++ b/skill-assets/fetch-asset.cjs @@ -27,6 +27,8 @@ const os = require('os') const path = require('path') const dns = require('dns').promises const net = require('net') +const { pipeline } = require('stream/promises') +const { Readable } = require('stream') const EXTENSIONS = { cpuprofile: '.cpuprofile', @@ -352,7 +354,7 @@ async function readCredentials () { return { consoleUrl: consoleUrl.replace(/\/$/, ''), token } } -async function fetchAsset (consoleUrl, token, assetId) { +async function downloadAsset (consoleUrl, token, assetId, destPath) { const url = `${consoleUrl}/api/v3/asset/${encodeURIComponent(assetId)}` console.log(`Fetching asset from: ${url}`) @@ -361,14 +363,19 @@ async function fetchAsset (consoleUrl, token, assetId) { 'x-nsolid-service-token': token, Accept: 'application/json' }, - signal: AbortSignal.timeout(120_000) + // 10-minute total budget: large snapshots (>256MB) over slow links + // must not abort at 120s mid-body. + signal: AbortSignal.timeout(600_000) }) if (!res.ok) { throw new Error(`Console returned ${res.status} ${res.statusText} for asset ${assetId}`) } - return await res.text() + // Stream body straight to disk — constant memory regardless of asset size. + // Node's fetch transparently decompresses Content-Encoding: gzip. + await pipeline(Readable.fromWeb(res.body), fs.createWriteStream(destPath)) + return fs.statSync(destPath).size } async function main () { @@ -398,9 +405,7 @@ async function main () { if (existingAsset.exists) { fileSize = fs.statSync(existingAsset.filePath).size } else { - const data = await fetchAsset(consoleUrl, token, assetId) - fs.writeFileSync(existingAsset.filePath, data, 'utf-8') - fileSize = Buffer.byteLength(data) + fileSize = await downloadAsset(consoleUrl, token, assetId, existingAsset.filePath) } // Register in .nsolid/assets/index.json so the extension's AssetService can discover it @@ -433,4 +438,4 @@ if (require.main === module) { }) } -module.exports = { isPrivateOrLocalIp, resolveHostnameIps, validateConsoleUrl, readCredentials, fetchAsset } +module.exports = { isPrivateOrLocalIp, resolveHostnameIps, validateConsoleUrl, readCredentials, downloadAsset } diff --git a/skills/ns-advanced-memory-leak-hunter/SKILL.md b/skills/ns-advanced-memory-leak-hunter/SKILL.md index 3ea1961..63b0eb5 100644 --- a/skills/ns-advanced-memory-leak-hunter/SKILL.md +++ b/skills/ns-advanced-memory-leak-hunter/SKILL.md @@ -100,4 +100,5 @@ description: >- ## Guardrails - **No early assumptions**: Never declare a memory leak from a single snapshot. Always compare baseline to peak. - **Reuse what exists**: Do not capture a new baseline or peak sample if the user already supplied the needed assets. -- **Wait times**: Memory tools block the thread. Do not spam endpoints while an asset is in progress. \ No newline at end of file +- **Wait times**: Memory tools block the thread. Do not spam endpoints while an asset is in progress. +- Never use the MCP `asset` tool to download raw assets (still exposed by older console versions); always use the bundled `fetch-asset.cjs`. \ No newline at end of file diff --git a/skills/ns-advanced-memory-leak-hunter/fetch-asset.cjs b/skills/ns-advanced-memory-leak-hunter/fetch-asset.cjs index b41c216..932dfe9 100644 --- a/skills/ns-advanced-memory-leak-hunter/fetch-asset.cjs +++ b/skills/ns-advanced-memory-leak-hunter/fetch-asset.cjs @@ -27,6 +27,8 @@ const os = require('os') const path = require('path') const dns = require('dns').promises const net = require('net') +const { pipeline } = require('stream/promises') +const { Readable } = require('stream') const EXTENSIONS = { cpuprofile: '.cpuprofile', @@ -352,7 +354,7 @@ async function readCredentials () { return { consoleUrl: consoleUrl.replace(/\/$/, ''), token } } -async function fetchAsset (consoleUrl, token, assetId) { +async function downloadAsset (consoleUrl, token, assetId, destPath) { const url = `${consoleUrl}/api/v3/asset/${encodeURIComponent(assetId)}` console.log(`Fetching asset from: ${url}`) @@ -361,14 +363,19 @@ async function fetchAsset (consoleUrl, token, assetId) { 'x-nsolid-service-token': token, Accept: 'application/json' }, - signal: AbortSignal.timeout(120_000) + // 10-minute total budget: large snapshots (>256MB) over slow links + // must not abort at 120s mid-body. + signal: AbortSignal.timeout(600_000) }) if (!res.ok) { throw new Error(`Console returned ${res.status} ${res.statusText} for asset ${assetId}`) } - return await res.text() + // Stream body straight to disk — constant memory regardless of asset size. + // Node's fetch transparently decompresses Content-Encoding: gzip. + await pipeline(Readable.fromWeb(res.body), fs.createWriteStream(destPath)) + return fs.statSync(destPath).size } async function main () { @@ -398,9 +405,7 @@ async function main () { if (existingAsset.exists) { fileSize = fs.statSync(existingAsset.filePath).size } else { - const data = await fetchAsset(consoleUrl, token, assetId) - fs.writeFileSync(existingAsset.filePath, data, 'utf-8') - fileSize = Buffer.byteLength(data) + fileSize = await downloadAsset(consoleUrl, token, assetId, existingAsset.filePath) } // Register in .nsolid/assets/index.json so the extension's AssetService can discover it @@ -433,4 +438,4 @@ if (require.main === module) { }) } -module.exports = { isPrivateOrLocalIp, resolveHostnameIps, validateConsoleUrl, readCredentials, fetchAsset } +module.exports = { isPrivateOrLocalIp, resolveHostnameIps, validateConsoleUrl, readCredentials, downloadAsset } diff --git a/skills/ns-analyze-asset/SKILL.md b/skills/ns-analyze-asset/SKILL.md index 6093a40..e665850 100644 --- a/skills/ns-analyze-asset/SKILL.md +++ b/skills/ns-analyze-asset/SKILL.md @@ -127,3 +127,4 @@ Rules: ## Guardrails - Never analyze a heap snapshot that is still marked as processing or pending. Poll and wait until `asset-summary` returns the actual summarized content. - Do not lie about findings. If the asset summary lacks detail, say so explicitly. +- Never use the MCP `asset` tool to download raw assets (still exposed by older console versions); always use the bundled `fetch-asset.cjs`. diff --git a/skills/ns-analyze-asset/fetch-asset.cjs b/skills/ns-analyze-asset/fetch-asset.cjs index b41c216..932dfe9 100644 --- a/skills/ns-analyze-asset/fetch-asset.cjs +++ b/skills/ns-analyze-asset/fetch-asset.cjs @@ -27,6 +27,8 @@ const os = require('os') const path = require('path') const dns = require('dns').promises const net = require('net') +const { pipeline } = require('stream/promises') +const { Readable } = require('stream') const EXTENSIONS = { cpuprofile: '.cpuprofile', @@ -352,7 +354,7 @@ async function readCredentials () { return { consoleUrl: consoleUrl.replace(/\/$/, ''), token } } -async function fetchAsset (consoleUrl, token, assetId) { +async function downloadAsset (consoleUrl, token, assetId, destPath) { const url = `${consoleUrl}/api/v3/asset/${encodeURIComponent(assetId)}` console.log(`Fetching asset from: ${url}`) @@ -361,14 +363,19 @@ async function fetchAsset (consoleUrl, token, assetId) { 'x-nsolid-service-token': token, Accept: 'application/json' }, - signal: AbortSignal.timeout(120_000) + // 10-minute total budget: large snapshots (>256MB) over slow links + // must not abort at 120s mid-body. + signal: AbortSignal.timeout(600_000) }) if (!res.ok) { throw new Error(`Console returned ${res.status} ${res.statusText} for asset ${assetId}`) } - return await res.text() + // Stream body straight to disk — constant memory regardless of asset size. + // Node's fetch transparently decompresses Content-Encoding: gzip. + await pipeline(Readable.fromWeb(res.body), fs.createWriteStream(destPath)) + return fs.statSync(destPath).size } async function main () { @@ -398,9 +405,7 @@ async function main () { if (existingAsset.exists) { fileSize = fs.statSync(existingAsset.filePath).size } else { - const data = await fetchAsset(consoleUrl, token, assetId) - fs.writeFileSync(existingAsset.filePath, data, 'utf-8') - fileSize = Buffer.byteLength(data) + fileSize = await downloadAsset(consoleUrl, token, assetId, existingAsset.filePath) } // Register in .nsolid/assets/index.json so the extension's AssetService can discover it @@ -433,4 +438,4 @@ if (require.main === module) { }) } -module.exports = { isPrivateOrLocalIp, resolveHostnameIps, validateConsoleUrl, readCredentials, fetchAsset } +module.exports = { isPrivateOrLocalIp, resolveHostnameIps, validateConsoleUrl, readCredentials, downloadAsset } diff --git a/skills/ns-cpu-spike-analysis/SKILL.md b/skills/ns-cpu-spike-analysis/SKILL.md index bd217fe..1c401ab 100644 --- a/skills/ns-cpu-spike-analysis/SKILL.md +++ b/skills/ns-cpu-spike-analysis/SKILL.md @@ -140,6 +140,7 @@ description: >- - NEVER ask for capture approval on a telemetry alert unless the user explicitly requested read-only/offline behavior. - NEVER call discovery tools only to restate the same app and spike value the user already provided; continue to the target-agent selection and profile. - ALWAYS wait via the bundled `wait.cjs` script, never `setTimeout`/`sleep` or an estimate. Download assets via the bundled `fetch-asset.cjs` (do not use direct HTTP calls or curl). The only other shell helper allowed by this skill is the bundled same-directory `workspace-delta.cjs`. +- Never use the MCP `asset` tool to download raw assets (still exposed by older console versions); always use the bundled `fetch-asset.cjs`. - NEVER paste the entire `runtime-code` response when only part of it is relevant. Keep the report focused on the code that explains the problem and proposed fix. - NEVER fetch or present dependency or Node-internal source as the code to optimize. Treat those frames as evidence, then explain the nearest relevant user-owned caller instead. - If you do not wait long enough with `wait.cjs`, `asset-summary` may still report that the profile asset is not ready. diff --git a/skills/ns-cpu-spike-analysis/fetch-asset.cjs b/skills/ns-cpu-spike-analysis/fetch-asset.cjs index b41c216..932dfe9 100644 --- a/skills/ns-cpu-spike-analysis/fetch-asset.cjs +++ b/skills/ns-cpu-spike-analysis/fetch-asset.cjs @@ -27,6 +27,8 @@ const os = require('os') const path = require('path') const dns = require('dns').promises const net = require('net') +const { pipeline } = require('stream/promises') +const { Readable } = require('stream') const EXTENSIONS = { cpuprofile: '.cpuprofile', @@ -352,7 +354,7 @@ async function readCredentials () { return { consoleUrl: consoleUrl.replace(/\/$/, ''), token } } -async function fetchAsset (consoleUrl, token, assetId) { +async function downloadAsset (consoleUrl, token, assetId, destPath) { const url = `${consoleUrl}/api/v3/asset/${encodeURIComponent(assetId)}` console.log(`Fetching asset from: ${url}`) @@ -361,14 +363,19 @@ async function fetchAsset (consoleUrl, token, assetId) { 'x-nsolid-service-token': token, Accept: 'application/json' }, - signal: AbortSignal.timeout(120_000) + // 10-minute total budget: large snapshots (>256MB) over slow links + // must not abort at 120s mid-body. + signal: AbortSignal.timeout(600_000) }) if (!res.ok) { throw new Error(`Console returned ${res.status} ${res.statusText} for asset ${assetId}`) } - return await res.text() + // Stream body straight to disk — constant memory regardless of asset size. + // Node's fetch transparently decompresses Content-Encoding: gzip. + await pipeline(Readable.fromWeb(res.body), fs.createWriteStream(destPath)) + return fs.statSync(destPath).size } async function main () { @@ -398,9 +405,7 @@ async function main () { if (existingAsset.exists) { fileSize = fs.statSync(existingAsset.filePath).size } else { - const data = await fetchAsset(consoleUrl, token, assetId) - fs.writeFileSync(existingAsset.filePath, data, 'utf-8') - fileSize = Buffer.byteLength(data) + fileSize = await downloadAsset(consoleUrl, token, assetId, existingAsset.filePath) } // Register in .nsolid/assets/index.json so the extension's AssetService can discover it @@ -433,4 +438,4 @@ if (require.main === module) { }) } -module.exports = { isPrivateOrLocalIp, resolveHostnameIps, validateConsoleUrl, readCredentials, fetchAsset } +module.exports = { isPrivateOrLocalIp, resolveHostnameIps, validateConsoleUrl, readCredentials, downloadAsset } diff --git a/skills/ns-download-asset/SKILL.md b/skills/ns-download-asset/SKILL.md new file mode 100644 index 0000000..860369b --- /dev/null +++ b/skills/ns-download-asset/SKILL.md @@ -0,0 +1,44 @@ +--- +name: ns-download-asset +description: >- + Downloads a raw N|Solid diagnostic asset (heap snapshot, CPU profile, or heap profile) to local disk by asset ID. Use when the user asks to download, save, export, or pull an asset file locally, or needs the raw file for external tools such as snapshot-parser, Chrome DevTools, or diffing. Do not use for analyzing assets — ns-analyze-asset covers that. +--- + +## Instructions + +### 1. Identify the Asset + +- The asset ID comes from the user or from the `assets` MCP tool (filter with `type`, `app`, etc.). +- If the user gives only an app name + asset type, call `assets` and confirm which asset to download before proceeding. +- If the asset was just generated, make sure it is no longer listed in `assets-in-progress` before downloading. + +### 2. Resolve the Asset Type + +- Map the asset to the script's `assetType`: `cpuprofile` | `heapprofile` | `heapsnapshot`. +- Heap sampling assets use `heapprofile`. + +### 3. Resolve the App Name + +- Resolve `appName` from the `assets` metadata or the user. +- Fall back to `unknown` only when the app name is genuinely unknown. + +### 4. Download + +Run the bundled script (use the absolute path of the directory where you read this SKILL.md): + +``` +node "/fetch-asset.cjs" +``` + +### 5. Report + +- Output the file path under `.nsolid/assets/`, the size printed by the script, and that the asset is registered in `.nsolid/assets/index.json`. +- Never open or read the raw file into context. + +## Guardrails + +- Never download assets via direct HTTP calls, `curl`, or ad hoc shell — only the bundled `fetch-asset.cjs`. +- Never use the MCP `asset` tool even if it appears in the tool list (older console versions still expose it). It is deprecated: it inlines raw data into the response and kills the MCP session with "session expired" on large assets. Always `fetch-asset.cjs`. +- Never read raw asset contents into the AI context — the file is for external tools and the user. +- The script dedupes by asset ID (existing files are reused); do not re-download unnecessarily. +- Do not fabricate analysis from the downloaded file — analyzing assets is `ns-analyze-asset`'s job via `asset-summary`. diff --git a/skills/ns-download-asset/fetch-asset.cjs b/skills/ns-download-asset/fetch-asset.cjs new file mode 100644 index 0000000..932dfe9 --- /dev/null +++ b/skills/ns-download-asset/fetch-asset.cjs @@ -0,0 +1,441 @@ +#!/usr/bin/env node + +// fetch-asset.cjs — Downloads a full N|Solid asset (CPU profile, heap snapshot, +// heap sampling) from the console API and saves it to .nsolid/assets/. +// +// Usage: +// node fetch-asset.cjs [appName] +// +// Arguments: +// assetId — The asset ID returned by the profile/snapshot/heap-sampling MCP tool +// assetType — One of: cpuprofile, heapprofile, heapsnapshot +// appName — (Optional) Application name for the filename, defaults to "unknown" +// +// The script reads the console URL and service token from ~/.agents/.nodesource-auth.json. +// Assets are saved to /.nsolid/assets/. +// +// Output files: +// .nsolid/assets/--. +// +// Note: This script is designed for single-process/single-user workflows. +// Concurrent executions may race on file operations and index updates. + +'use strict' + +const fs = require('fs') +const os = require('os') +const path = require('path') +const dns = require('dns').promises +const net = require('net') +const { pipeline } = require('stream/promises') +const { Readable } = require('stream') + +const EXTENSIONS = { + cpuprofile: '.cpuprofile', + heapprofile: '.heapprofile', + heapsnapshot: '.heapsnapshot' +} + +// Maps fetch-asset type args to the AssetType values used by the extension's AssetService +const ASSET_TYPES = { + cpuprofile: 'cpu-profile', + heapprofile: 'heap-profile', + heapsnapshot: 'heap-snapshot' +} + +function getAssetsDir (workspaceRoot) { + return path.join(workspaceRoot, '.nsolid', 'assets') +} + +function sanitizeAppName (appName) { + return appName.replace(/[^a-zA-Z0-9_-]/g, '_') +} + +function buildAssetFilename (assetType, appName, assetId) { + return `${assetType}-${sanitizeAppName(appName)}-${assetId.slice(0, 8)}${EXTENSIONS[assetType]}` +} + +function readAssetIndex (workspaceRoot) { + const indexPath = path.join(getAssetsDir(workspaceRoot), 'index.json') + + // Only treat a genuinely missing index as empty. A file that exists but is + // unreadable or malformed must NOT be swallowed into [] — otherwise the next + // saveToAssetIndex() upsert would overwrite it and silently drop every + // existing entry. Surface that error so the caller can fail loudly. + if (!fs.existsSync(indexPath)) { + return [] + } + + const raw = fs.readFileSync(indexPath, 'utf-8') + const parsed = JSON.parse(raw) + if (!Array.isArray(parsed)) { + throw new Error(`index.json is not an array; refusing to overwrite a malformed index at ${indexPath}`) + } + return parsed +} + +function isPathWithin (parent, candidate) { + const rel = path.relative(parent, candidate) + return rel !== '' && !rel.startsWith('..') && !path.isAbsolute(rel) +} + +function writeAssetIndex (workspaceRoot, records) { + const indexPath = path.join(getAssetsDir(workspaceRoot), 'index.json') + fs.writeFileSync(indexPath, JSON.stringify(records, null, 2), 'utf-8') +} + +function saveToAssetIndex (workspaceRoot, record) { + const records = readAssetIndex(workspaceRoot) + + // Upsert by assetId (mirrors AssetService.saveToIndex) + const idx = records.findIndex(r => r.assetId === record.assetId) + if (idx >= 0) { + records[idx] = record + } else { + records.push(record) + } + + writeAssetIndex(workspaceRoot, records) +} + +function removeDirectoryIfEmpty (dirPath) { + if (!fs.existsSync(dirPath)) { + return + } + + if (fs.readdirSync(dirPath).length === 0) { + fs.rmdirSync(dirPath) + } +} + +function resolveExistingAsset (workspaceRoot, assetId, assetType, appName) { + const assetsDir = getAssetsDir(workspaceRoot) + const expectedFilename = buildAssetFilename(assetType, appName, assetId) + const expectedPath = path.join(assetsDir, expectedFilename) + + if (fs.existsSync(expectedPath)) { + return { + filePath: expectedPath, + localPath: expectedFilename, + source: 'flat' + } + } + + // Lookups are lenient: a malformed/unreadable index.json must not block the + // legacy fallback probe below. readAssetIndex() stays strict for the write + // path (saveToAssetIndex) so a bad index is never silently clobbered; here + // we only need a best-effort match and treat any read failure as "no match". + let indexRecord + try { + indexRecord = readAssetIndex(workspaceRoot).find(record => record.assetId === assetId) + } catch { + indexRecord = undefined + } + if (indexRecord?.localPath) { + const indexedPath = path.resolve(assetsDir, indexRecord.localPath) + if (isPathWithin(assetsDir, indexedPath) && fs.existsSync(indexedPath)) { + return { + filePath: indexedPath, + localPath: indexRecord.localPath, + source: 'index' + } + } + } + + const legacyPath = path.join(assetsDir, sanitizeAppName(appName), `${assetId}${EXTENSIONS[assetType]}`) + if (fs.existsSync(legacyPath)) { + return { + filePath: legacyPath, + localPath: path.join(sanitizeAppName(appName), `${assetId}${EXTENSIONS[assetType]}`), + source: 'legacy' + } + } + + return null +} + +function ensureFlatAsset (workspaceRoot, assetId, assetType, appName) { + const assetsDir = getAssetsDir(workspaceRoot) + const filename = buildAssetFilename(assetType, appName, assetId) + const filePath = path.join(assetsDir, filename) + const existing = resolveExistingAsset(workspaceRoot, assetId, assetType, appName) + + if (!existing) { + return { + exists: false, + filePath, + localPath: filename + } + } + + if (existing.filePath !== filePath) { + fs.renameSync(existing.filePath, filePath) + if (existing.source === 'legacy') { + removeDirectoryIfEmpty(path.dirname(existing.filePath)) + } + + return { + exists: true, + migrated: true, + filePath, + localPath: filename + } + } + + return { + exists: true, + migrated: false, + filePath, + localPath: filename + } +} + +function expandIPv6 (ip) { + // IPv4-mapped/compatible addresses embed a dotted IPv4 at the end and must + // not be expanded with the generic :: handler below. + const embeddedIpv4 = extractIpv4FromIpv6(ip) + if (embeddedIpv4) { + return null + } + + let expanded = ip.toLowerCase() + if (expanded.includes('::')) { + const [left, right] = expanded.split('::') + const leftParts = left ? left.split(':') : [] + const rightParts = right ? right.split(':') : [] + const missing = 8 - leftParts.length - rightParts.length + const middle = Array(Math.max(missing, 0)).fill('0000') + expanded = [...leftParts, ...middle, ...rightParts].join(':') + } + return expanded.split(':').map(p => p.padStart(4, '0')).join(':') +} + +function extractIpv4FromIpv6 (ip) { + // IPv4-mapped: ::ffff:a.b.c.d or 0:0:0:0:0:ffff:a.b.c.d + const mapped = ip.match(/^(?:::|(?:0:){5})ffff:(\d+\.\d+\.\d+\.\d+)$/i) + if (mapped) return mapped[1] + // IPv4-compatible: ::a.b.c.d or 0:0:0:0:0:0:a.b.c.d + const compatible = ip.match(/^(?:::|(?:0:){6})(\d+\.\d+\.\d+\.\d+)$/i) + if (compatible) return compatible[1] + return null +} + +function isPrivateOrLocalIp (ip) { + if (net.isIPv4(ip)) { + const [a, b] = ip.split('.').map(Number) + if (a === 127) return true // loopback 127.0.0.0/8 + if (a === 10) return true // private 10.0.0.0/8 + if (a === 172 && b >= 16 && b <= 31) return true // private 172.16.0.0/12 + if (a === 192 && b === 168) return true // private 192.168.0.0/16 + if (a === 169 && b === 254) return true // link-local 169.254.0.0/16 + if (a === 0) return true // current network 0.0.0.0/8 + return false + } + + if (net.isIPv6(ip)) { + const embeddedIpv4 = extractIpv4FromIpv6(ip) + if (embeddedIpv4) { + return isPrivateOrLocalIp(embeddedIpv4) + } + + const normalized = expandIPv6(ip) + if (normalized === null) { + // Defensive: extractIpv4FromIpv6 should have matched any mapped/compatible + // address that net.isIPv6 accepted, but treat unexpected forms as unsafe. + return true + } + + // URL parsers normalize IPv4-mapped (::ffff:a.b.c.d) and IPv4-compatible + // (::a.b.c.d) addresses to pure hex. Detect those forms by prefix. + if (normalized.startsWith('0000:0000:0000:0000:0000:ffff:') || + normalized.startsWith('0000:0000:0000:0000:0000:0000:')) { + const high = parseInt(normalized.slice(30, 34), 16) + const low = parseInt(normalized.slice(35, 39), 16) + const ipv4 = `${(high >> 8) & 0xff}.${high & 0xff}.${(low >> 8) & 0xff}.${low & 0xff}` + return isPrivateOrLocalIp(ipv4) + } + + const first16 = parseInt(normalized.slice(0, 4), 16) + if (normalized === '0000:0000:0000:0000:0000:0000:0000:0001') return true // ::1 + if ((first16 & 0xffc0) === 0xfe80) return true // link-local fe80::/10 + if ((first16 & 0xfe00) === 0xfc00) return true // unique local fc00::/7 + return false + } + + return false +} + +async function resolveHostnameIps (hostname) { + const raw = hostname.replace(/^\[/, '').replace(/\]$/, '') + const ipVersion = net.isIP(raw) + + if (ipVersion === 4) { + return [raw] + } + if (ipVersion === 6) { + return [raw] + } + + const ips = [] + // Use dns.lookup (libuv/getaddrinfo), which honors /etc/hosts and the + // system resolver — not dns.resolve (c-ares), which bypasses /etc/hosts and + // therefore fails to resolve hostnames like `localhost` on platforms where + // they only exist in the hosts file (e.g. macOS). This also matches the real + // resolution an outbound fetch would use, which is what SSRF validation needs. + try { + const records = await dns.lookup(raw, { all: true, verbatim: true }) + ips.push(...records.map((r) => r.address)) + } catch { + // hostname could not be resolved; caller treats empty as an error + } + return ips +} + +async function validateConsoleUrl (consoleUrl) { + let url + try { + url = new URL(consoleUrl) + } catch { + throw new Error(`Invalid consoleUrl: ${consoleUrl}`) + } + + if (process.env.NSOLID_ALLOW_INSECURE_CONSOLE) { + return + } + + if (url.protocol !== 'https:') { + throw new Error(`consoleUrl must use HTTPS: ${consoleUrl}`) + } + + const hostname = url.hostname.toLowerCase().replace(/\.$/, '') + if (hostname === 'localhost' || hostname === '127.0.0.1' || hostname === '[::1]' || hostname === '::1') { + throw new Error(`consoleUrl cannot be localhost: ${consoleUrl}`) + } + + const ips = await resolveHostnameIps(url.hostname) + if (ips.length === 0) { + throw new Error(`consoleUrl hostname could not be resolved: ${consoleUrl}`) + } + + for (const ip of ips) { + if (isPrivateOrLocalIp(ip)) { + throw new Error(`consoleUrl resolves to a private or local address: ${consoleUrl} (${ip})`) + } + } +} + +async function readCredentials () { + const authPath = path.join(os.homedir(), '.agents', '.nodesource-auth.json') + + if (!fs.existsSync(authPath)) { + throw new Error( + 'Credentials not found. Run "npx @nodesource/plugin- login" to authenticate.' + ) + } + + let auth + try { + auth = JSON.parse(fs.readFileSync(authPath, 'utf-8')) + } catch (e) { + throw new Error(`Failed to parse ${authPath}: ${e.message}`) + } + const consoleUrl = auth.consoleUrl + const token = auth.serviceToken + + if (!consoleUrl) { + throw new Error('Missing "consoleUrl" in ~/.agents/.nodesource-auth.json') + } + if (!token) { + throw new Error('Missing "serviceToken" in ~/.agents/.nodesource-auth.json') + } + + await validateConsoleUrl(consoleUrl) + + return { consoleUrl: consoleUrl.replace(/\/$/, ''), token } +} + +async function downloadAsset (consoleUrl, token, assetId, destPath) { + const url = `${consoleUrl}/api/v3/asset/${encodeURIComponent(assetId)}` + console.log(`Fetching asset from: ${url}`) + + const res = await fetch(url, { + headers: { + 'x-nsolid-service-token': token, + Accept: 'application/json' + }, + // 10-minute total budget: large snapshots (>256MB) over slow links + // must not abort at 120s mid-body. + signal: AbortSignal.timeout(600_000) + }) + + if (!res.ok) { + throw new Error(`Console returned ${res.status} ${res.statusText} for asset ${assetId}`) + } + + // Stream body straight to disk — constant memory regardless of asset size. + // Node's fetch transparently decompresses Content-Encoding: gzip. + await pipeline(Readable.fromWeb(res.body), fs.createWriteStream(destPath)) + return fs.statSync(destPath).size +} + +async function main () { + const [,, assetId, assetType, appName = 'unknown'] = process.argv + + if (!assetId || !assetType) { + console.error('Usage: node fetch-asset.cjs [appName]') + console.error(' assetType: cpuprofile | heapprofile | heapsnapshot') + process.exit(1) + } + + const ext = EXTENSIONS[assetType] + if (!ext) { + console.error(`Unknown asset type: ${assetType}. Use one of: ${Object.keys(EXTENSIONS).join(', ')}`) + process.exit(1) + } + + const workspaceRoot = process.cwd() + const { consoleUrl, token } = await readCredentials() + + const assetsDir = getAssetsDir(workspaceRoot) + fs.mkdirSync(assetsDir, { recursive: true }) + + const existingAsset = ensureFlatAsset(workspaceRoot, assetId, assetType, appName) + + let fileSize + if (existingAsset.exists) { + fileSize = fs.statSync(existingAsset.filePath).size + } else { + fileSize = await downloadAsset(consoleUrl, token, assetId, existingAsset.filePath) + } + + // Register in .nsolid/assets/index.json so the extension's AssetService can discover it + saveToAssetIndex(workspaceRoot, { + assetId, + name: `${assetType}-${sanitizeAppName(appName)}-${assetId.slice(0, 8)}`, + type: ASSET_TYPES[assetType], + app: appName, + localPath: existingAsset.localPath, + downloadedAt: new Date().toISOString(), + fileSize + }) + + if (existingAsset.exists) { + if (existingAsset.migrated) { + console.log(`Asset already existed and was moved to: ${existingAsset.filePath}`) + } else { + console.log(`Asset already downloaded at: ${existingAsset.filePath}`) + } + } else { + console.log(`Asset saved to: ${existingAsset.filePath}`) + } + console.log(`File size: ${(fileSize / 1024).toFixed(1)} KB`) +} + +if (require.main === module) { + main().catch((err) => { + console.error(`Error: ${err.message}`) + process.exit(1) + }) +} + +module.exports = { isPrivateOrLocalIp, resolveHostnameIps, validateConsoleUrl, readCredentials, downloadAsset } diff --git a/skills/ns-generate-asset/SKILL.md b/skills/ns-generate-asset/SKILL.md index 76f3581..e86f255 100644 --- a/skills/ns-generate-asset/SKILL.md +++ b/skills/ns-generate-asset/SKILL.md @@ -62,4 +62,5 @@ Run the bundled wait script (use the absolute path of the directory where you re ## Guardrails - Do not use `runtime-code` or `workspace_delta`. - Waits and downloads use the bundled `wait.cjs` and `fetch-asset.cjs` scripts in this skill's directory. Do not use direct HTTP calls, `curl`, or ad hoc shell commands. +- Never use the MCP `asset` tool to download raw assets (still exposed by older console versions); always use the bundled `fetch-asset.cjs`. - Do not use `assets-in-progress` as the first readiness check for CPU profiles or heap samples. diff --git a/skills/ns-generate-asset/fetch-asset.cjs b/skills/ns-generate-asset/fetch-asset.cjs index b41c216..932dfe9 100644 --- a/skills/ns-generate-asset/fetch-asset.cjs +++ b/skills/ns-generate-asset/fetch-asset.cjs @@ -27,6 +27,8 @@ const os = require('os') const path = require('path') const dns = require('dns').promises const net = require('net') +const { pipeline } = require('stream/promises') +const { Readable } = require('stream') const EXTENSIONS = { cpuprofile: '.cpuprofile', @@ -352,7 +354,7 @@ async function readCredentials () { return { consoleUrl: consoleUrl.replace(/\/$/, ''), token } } -async function fetchAsset (consoleUrl, token, assetId) { +async function downloadAsset (consoleUrl, token, assetId, destPath) { const url = `${consoleUrl}/api/v3/asset/${encodeURIComponent(assetId)}` console.log(`Fetching asset from: ${url}`) @@ -361,14 +363,19 @@ async function fetchAsset (consoleUrl, token, assetId) { 'x-nsolid-service-token': token, Accept: 'application/json' }, - signal: AbortSignal.timeout(120_000) + // 10-minute total budget: large snapshots (>256MB) over slow links + // must not abort at 120s mid-body. + signal: AbortSignal.timeout(600_000) }) if (!res.ok) { throw new Error(`Console returned ${res.status} ${res.statusText} for asset ${assetId}`) } - return await res.text() + // Stream body straight to disk — constant memory regardless of asset size. + // Node's fetch transparently decompresses Content-Encoding: gzip. + await pipeline(Readable.fromWeb(res.body), fs.createWriteStream(destPath)) + return fs.statSync(destPath).size } async function main () { @@ -398,9 +405,7 @@ async function main () { if (existingAsset.exists) { fileSize = fs.statSync(existingAsset.filePath).size } else { - const data = await fetchAsset(consoleUrl, token, assetId) - fs.writeFileSync(existingAsset.filePath, data, 'utf-8') - fileSize = Buffer.byteLength(data) + fileSize = await downloadAsset(consoleUrl, token, assetId, existingAsset.filePath) } // Register in .nsolid/assets/index.json so the extension's AssetService can discover it @@ -433,4 +438,4 @@ if (require.main === module) { }) } -module.exports = { isPrivateOrLocalIp, resolveHostnameIps, validateConsoleUrl, readCredentials, fetchAsset } +module.exports = { isPrivateOrLocalIp, resolveHostnameIps, validateConsoleUrl, readCredentials, downloadAsset } diff --git a/skills/ns-memory-spike-analysis/SKILL.md b/skills/ns-memory-spike-analysis/SKILL.md index 2a5d397..f3fa265 100644 --- a/skills/ns-memory-spike-analysis/SKILL.md +++ b/skills/ns-memory-spike-analysis/SKILL.md @@ -82,3 +82,4 @@ Run the bundled wait script (use the absolute path of the directory where you re - Do not turn a user-supplied asset review into a fresh capture workflow unless the user asked for that or approved it. - Prioritize `heap-sampling` over `snapshot` to minimize production impact. - After a successful `asset-summary`, analyze that summary instead of falling back to a generic telemetry-only conclusion. +- Never use the MCP `asset` tool to download raw assets (still exposed by older console versions); always use the bundled `fetch-asset.cjs`. diff --git a/skills/ns-memory-spike-analysis/fetch-asset.cjs b/skills/ns-memory-spike-analysis/fetch-asset.cjs index b41c216..932dfe9 100644 --- a/skills/ns-memory-spike-analysis/fetch-asset.cjs +++ b/skills/ns-memory-spike-analysis/fetch-asset.cjs @@ -27,6 +27,8 @@ const os = require('os') const path = require('path') const dns = require('dns').promises const net = require('net') +const { pipeline } = require('stream/promises') +const { Readable } = require('stream') const EXTENSIONS = { cpuprofile: '.cpuprofile', @@ -352,7 +354,7 @@ async function readCredentials () { return { consoleUrl: consoleUrl.replace(/\/$/, ''), token } } -async function fetchAsset (consoleUrl, token, assetId) { +async function downloadAsset (consoleUrl, token, assetId, destPath) { const url = `${consoleUrl}/api/v3/asset/${encodeURIComponent(assetId)}` console.log(`Fetching asset from: ${url}`) @@ -361,14 +363,19 @@ async function fetchAsset (consoleUrl, token, assetId) { 'x-nsolid-service-token': token, Accept: 'application/json' }, - signal: AbortSignal.timeout(120_000) + // 10-minute total budget: large snapshots (>256MB) over slow links + // must not abort at 120s mid-body. + signal: AbortSignal.timeout(600_000) }) if (!res.ok) { throw new Error(`Console returned ${res.status} ${res.statusText} for asset ${assetId}`) } - return await res.text() + // Stream body straight to disk — constant memory regardless of asset size. + // Node's fetch transparently decompresses Content-Encoding: gzip. + await pipeline(Readable.fromWeb(res.body), fs.createWriteStream(destPath)) + return fs.statSync(destPath).size } async function main () { @@ -398,9 +405,7 @@ async function main () { if (existingAsset.exists) { fileSize = fs.statSync(existingAsset.filePath).size } else { - const data = await fetchAsset(consoleUrl, token, assetId) - fs.writeFileSync(existingAsset.filePath, data, 'utf-8') - fileSize = Buffer.byteLength(data) + fileSize = await downloadAsset(consoleUrl, token, assetId, existingAsset.filePath) } // Register in .nsolid/assets/index.json so the extension's AssetService can discover it @@ -433,4 +438,4 @@ if (require.main === module) { }) } -module.exports = { isPrivateOrLocalIp, resolveHostnameIps, validateConsoleUrl, readCredentials, fetchAsset } +module.exports = { isPrivateOrLocalIp, resolveHostnameIps, validateConsoleUrl, readCredentials, downloadAsset } From 7f8bf018479c0ba34b8fefaa1d7bca67a8d03a0d Mon Sep 17 00:00:00 2001 From: Cesar-M-Diaz Date: Fri, 21 Aug 2026 17:08:56 +0200 Subject: [PATCH 2/8] fix(skills): secure streamed asset downloads --- package.json | 3 +- packages/core/package.json | 1 + .../core/test/unit/skills/fetch-asset.test.ts | 68 +++++++++-- pnpm-lock.yaml | 14 ++- skill-assets/fetch-asset.cjs | 113 ++++++++++++++---- .../fetch-asset.cjs | 113 ++++++++++++++---- skills/ns-analyze-asset/fetch-asset.cjs | 113 ++++++++++++++---- skills/ns-cpu-spike-analysis/fetch-asset.cjs | 113 ++++++++++++++---- skills/ns-download-asset/SKILL.md | 2 +- skills/ns-download-asset/fetch-asset.cjs | 113 ++++++++++++++---- skills/ns-generate-asset/fetch-asset.cjs | 113 ++++++++++++++---- .../ns-memory-spike-analysis/fetch-asset.cjs | 113 ++++++++++++++---- 12 files changed, 710 insertions(+), 169 deletions(-) diff --git a/package.json b/package.json index 69fb67e..6aeb697 100644 --- a/package.json +++ b/package.json @@ -30,7 +30,8 @@ "eslint": "9.39.4", "husky": "9.1.7", "neostandard": "0.13.0", - "tsx": "4.22.4" + "tsx": "4.22.4", + "undici": "7.28.0" }, "dependencies": { "mcp-remote": "0.1.38" diff --git a/packages/core/package.json b/packages/core/package.json index 81fc2b8..87c4a78 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -31,6 +31,7 @@ }, "dependencies": { "smol-toml": "^1.3.1", + "undici": "7.28.0", "write-file-atomic": "^5.0.1", "zod": "4.4.3" }, diff --git a/packages/core/test/unit/skills/fetch-asset.test.ts b/packages/core/test/unit/skills/fetch-asset.test.ts index adf89d1..b3f3173 100644 --- a/packages/core/test/unit/skills/fetch-asset.test.ts +++ b/packages/core/test/unit/skills/fetch-asset.test.ts @@ -3,7 +3,7 @@ import assert from 'node:assert/strict' import { promises as dns } from 'node:dns' import { fileURLToPath, pathToFileURL } from 'node:url' import { dirname, join } from 'node:path' -import { mkdtemp, readFile, rm, stat } from 'node:fs/promises' +import { mkdtemp, readFile, readdir, rm, stat } from 'node:fs/promises' import { tmpdir } from 'node:os' import type { TestContext } from 'node:test' @@ -32,8 +32,8 @@ async function loadFetchAsset () { return mod as { isPrivateOrLocalIp: (ip: string) => boolean resolveHostnameIps: (hostname: string) => Promise - validateConsoleUrl: (consoleUrl: string) => Promise - downloadAsset: (consoleUrl: string, token: string, assetId: string, destPath: string) => Promise + validateConsoleUrl: (consoleUrl: string) => Promise + downloadAsset: (consoleUrl: string, token: string, assetId: string, destPath: string, validatedIps?: string[] | null) => Promise } } @@ -239,7 +239,7 @@ describe('validateConsoleUrl', () => { it('allows public hostnames', async (t) => { mockDnsLookup(t, ['93.184.216.34']) const { validateConsoleUrl } = await loadFetchAsset() - await assert.doesNotReject(() => validateConsoleUrl('https://example.com')) + assert.deepEqual(await validateConsoleUrl('https://example.com'), ['93.184.216.34']) }) it('rejects invalid URLs', async () => { @@ -264,16 +264,33 @@ describe('downloadAsset', () => { it('streams the asset body to disk and returns the file size', async (t) => { const payload = new Uint8Array([0, 1, 2, 3, 4, 5, 6, 7, 8, 9]) - const calls: Array<{ url: string, headers: Record }> = [] + const calls: Array<{ + url: string + headers: Record + redirect: RequestRedirect | undefined + dispatcher: unknown + }> = [] t.mock.method(globalThis, 'fetch', (async (input: RequestInfo | URL, init?: RequestInit) => { - calls.push({ url: String(input), headers: (init?.headers ?? {}) as Record }) + const options = init as RequestInit & { dispatcher?: unknown } + calls.push({ + url: String(input), + headers: (init?.headers ?? {}) as Record, + redirect: init?.redirect, + dispatcher: options.dispatcher, + }) return new Response(payload) }) as typeof fetch) const { downloadAsset } = await loadFetchAsset() const destPath = join(tempDir, 'heapsnapshot-test.heapsnapshot') - const size = await downloadAsset('https://console.example.test', 'secret-token', 'asset-id-1', destPath) + const size = await downloadAsset( + 'https://console.example.test', + 'secret-token', + 'asset-id-1', + destPath, + ['93.184.216.34'] + ) const stats = await stat(destPath) assert.equal(size, stats.size, 'returns the on-disk file size') @@ -283,6 +300,9 @@ describe('downloadAsset', () => { assert.equal(calls[0].url, 'https://console.example.test/api/v3/asset/asset-id-1') assert.equal(calls[0].headers['x-nsolid-service-token'], 'secret-token') assert.equal(calls[0].headers.Accept, 'application/json') + assert.equal(calls[0].redirect, 'error') + assert.ok(calls[0].dispatcher, 'uses a dispatcher pinned to validated addresses') + assert.deepEqual(await readdir(tempDir), ['heapsnapshot-test.heapsnapshot']) }) it('URL-encodes the asset ID in the request path', async (t) => { @@ -293,7 +313,13 @@ describe('downloadAsset', () => { }) as typeof fetch) const { downloadAsset } = await loadFetchAsset() - await downloadAsset('https://console.example.test', 'token', 'id/with space', join(tempDir, 'a.heapsnapshot')) + await downloadAsset( + 'https://console.example.test', + 'token', + 'id/with space', + join(tempDir, 'a.heapsnapshot'), + ['93.184.216.34'] + ) assert.equal(urls[0], 'https://console.example.test/api/v3/asset/id%2Fwith%20space') }) @@ -306,9 +332,33 @@ describe('downloadAsset', () => { const { downloadAsset } = await loadFetchAsset() const destPath = join(tempDir, 'missing.heapsnapshot') await assert.rejects( - () => downloadAsset('https://console.example.test', 'token', 'missing-id', destPath), + () => downloadAsset('https://console.example.test', 'token', 'missing-id', destPath, ['93.184.216.34']), /404 Not Found for asset missing-id/ ) await assert.rejects(() => stat(destPath), /ENOENT/) }) + + it('removes temporary bytes when the response stream fails', async (t) => { + let pullCount = 0 + const body = new ReadableStream({ + pull (controller) { + if (pullCount++ === 0) { + controller.enqueue(new Uint8Array([1, 2, 3])) + return + } + controller.error(new Error('stream interrupted')) + }, + }) + t.mock.method(globalThis, 'fetch', (async () => new Response(body)) as typeof fetch) + + const { downloadAsset } = await loadFetchAsset() + const destPath = join(tempDir, 'partial.heapsnapshot') + + await assert.rejects( + () => downloadAsset('https://console.example.test', 'token', 'partial-id', destPath, ['93.184.216.34']), + /stream interrupted/ + ) + await assert.rejects(() => stat(destPath), /ENOENT/) + assert.deepEqual(await readdir(tempDir), [], 'does not leave a temporary file behind') + }) }) diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 8135e57..457c831 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -24,12 +24,18 @@ importers: tsx: specifier: 4.22.4 version: 4.22.4 + undici: + specifier: 7.28.0 + version: 7.28.0 packages/core: dependencies: smol-toml: specifier: ^1.3.1 version: 1.6.1 + undici: + specifier: 7.28.0 + version: 7.28.0 write-file-atomic: specifier: ^5.0.1 version: 5.0.1 @@ -1446,8 +1452,8 @@ packages: undici-types@6.21.0: resolution: {integrity: sha512-iwDZqg0QAGrg9Rav5H4n0M64c3mkR59cJ6wQp+7C4nI0gsmExaedaYLNO44eT4AtBBwjbTiGPMlt2Md0T9H9JQ==} - undici@7.29.0: - resolution: {integrity: sha512-IDxfleLmmbSskfWSUATiN1nfn2rDuvnMOqb5CWR92iIfojA0Ud+ulOAAEQ57LPr9rWmsreUyf5lwyao+7GNNVw==} + undici@7.28.0: + resolution: {integrity: sha512-cRZYrTDwWznlnRiPjggAGxZXanty6M8RV1ff8Wm4LWXBp7/IG8v5DnOm74DtUBp9OONpK75YlPnIjQqX0dBDtA==} engines: {node: '>=20.18.1'} unicorn-magic@0.3.0: @@ -2664,7 +2670,7 @@ snapshots: express: 4.22.2 open: 10.2.0 strict-url-sanitise: 0.0.1 - undici: 7.29.0 + undici: 7.28.0 transitivePeerDependencies: - supports-color @@ -3151,7 +3157,7 @@ snapshots: undici-types@6.21.0: {} - undici@7.29.0: {} + undici@7.28.0: {} unicorn-magic@0.3.0: {} diff --git a/skill-assets/fetch-asset.cjs b/skill-assets/fetch-asset.cjs index 932dfe9..3f72628 100644 --- a/skill-assets/fetch-asset.cjs +++ b/skill-assets/fetch-asset.cjs @@ -27,8 +27,10 @@ const os = require('os') const path = require('path') const dns = require('dns').promises const net = require('net') +const { randomUUID } = require('crypto') const { pipeline } = require('stream/promises') const { Readable } = require('stream') +const { Agent } = require('undici') const EXTENSIONS = { cpuprofile: '.cpuprofile', @@ -300,7 +302,7 @@ async function validateConsoleUrl (consoleUrl) { } if (process.env.NSOLID_ALLOW_INSECURE_CONSOLE) { - return + return null } if (url.protocol !== 'https:') { @@ -322,6 +324,51 @@ async function validateConsoleUrl (consoleUrl) { throw new Error(`consoleUrl resolves to a private or local address: ${consoleUrl} (${ip})`) } } + + return ips +} + +function createPinnedDispatcher (consoleUrl, resolvedIps) { + const expectedHostname = new URL(consoleUrl).hostname.toLowerCase().replace(/^\[/, '').replace(/\]$/, '').replace(/\.$/, '') + const allowedIps = [...new Set(resolvedIps)] + + if (allowedIps.length === 0) { + throw new Error(`No validated addresses available for consoleUrl: ${consoleUrl}`) + } + + return new Agent({ + connect: { + autoSelectFamily: true, + lookup: (hostname, options, callback) => { + const requestedHostname = hostname.toLowerCase().replace(/\.$/, '') + if (requestedHostname !== expectedHostname) { + const error = new Error(`Refusing to resolve unvalidated hostname: ${hostname}`) + error.code = 'ENOTFOUND' + callback(error) + return + } + + const family = typeof options === 'number' ? options : options?.family + const matches = allowedIps + .map(address => ({ address, family: net.isIP(address) })) + .filter(record => record.family !== 0 && (!family || record.family === family)) + + if (matches.length === 0) { + const error = new Error(`No validated address matches the requested family for: ${hostname}`) + error.code = 'ENOTFOUND' + callback(error) + return + } + + if (typeof options === 'object' && options?.all) { + callback(null, matches) + return + } + + callback(null, matches[0].address, matches[0].family) + } + } + }) } async function readCredentials () { @@ -349,33 +396,55 @@ async function readCredentials () { throw new Error('Missing "serviceToken" in ~/.agents/.nodesource-auth.json') } - await validateConsoleUrl(consoleUrl) + const resolvedIps = await validateConsoleUrl(consoleUrl) - return { consoleUrl: consoleUrl.replace(/\/$/, ''), token } + return { consoleUrl: consoleUrl.replace(/\/$/, ''), token, resolvedIps } } -async function downloadAsset (consoleUrl, token, assetId, destPath) { +async function downloadAsset (consoleUrl, token, assetId, destPath, validatedIps) { const url = `${consoleUrl}/api/v3/asset/${encodeURIComponent(assetId)}` console.log(`Fetching asset from: ${url}`) - const res = await fetch(url, { - headers: { - 'x-nsolid-service-token': token, - Accept: 'application/json' - }, - // 10-minute total budget: large snapshots (>256MB) over slow links - // must not abort at 120s mid-body. - signal: AbortSignal.timeout(600_000) - }) + const resolvedIps = validatedIps === undefined + ? await validateConsoleUrl(consoleUrl) + : validatedIps + const dispatcher = resolvedIps === null + ? undefined + : createPinnedDispatcher(consoleUrl, resolvedIps) - if (!res.ok) { - throw new Error(`Console returned ${res.status} ${res.statusText} for asset ${assetId}`) - } + try { + const res = await fetch(url, { + headers: { + 'x-nsolid-service-token': token, + Accept: 'application/json' + }, + redirect: 'error', + dispatcher, + // 10-minute total budget: large snapshots (>256MB) over slow links + // must not abort at 120s mid-body. + signal: AbortSignal.timeout(600_000) + }) + + if (!res.ok) { + await res.body?.cancel() + throw new Error(`Console returned ${res.status} ${res.statusText} for asset ${assetId}`) + } + + // Keep incomplete bytes invisible to resolveExistingAsset(). The temporary + // file lives beside the destination so renameSync publishes it atomically. + const tempPath = `${destPath}.${process.pid}.${randomUUID()}.tmp` + try { + await pipeline(Readable.fromWeb(res.body), fs.createWriteStream(tempPath, { flags: 'wx' })) + fs.renameSync(tempPath, destPath) + } catch (error) { + fs.rmSync(tempPath, { force: true }) + throw error + } - // Stream body straight to disk — constant memory regardless of asset size. - // Node's fetch transparently decompresses Content-Encoding: gzip. - await pipeline(Readable.fromWeb(res.body), fs.createWriteStream(destPath)) - return fs.statSync(destPath).size + return fs.statSync(destPath).size + } finally { + await dispatcher?.close() + } } async function main () { @@ -394,7 +463,7 @@ async function main () { } const workspaceRoot = process.cwd() - const { consoleUrl, token } = await readCredentials() + const { consoleUrl, token, resolvedIps } = await readCredentials() const assetsDir = getAssetsDir(workspaceRoot) fs.mkdirSync(assetsDir, { recursive: true }) @@ -405,7 +474,7 @@ async function main () { if (existingAsset.exists) { fileSize = fs.statSync(existingAsset.filePath).size } else { - fileSize = await downloadAsset(consoleUrl, token, assetId, existingAsset.filePath) + fileSize = await downloadAsset(consoleUrl, token, assetId, existingAsset.filePath, resolvedIps) } // Register in .nsolid/assets/index.json so the extension's AssetService can discover it diff --git a/skills/ns-advanced-memory-leak-hunter/fetch-asset.cjs b/skills/ns-advanced-memory-leak-hunter/fetch-asset.cjs index 932dfe9..3f72628 100644 --- a/skills/ns-advanced-memory-leak-hunter/fetch-asset.cjs +++ b/skills/ns-advanced-memory-leak-hunter/fetch-asset.cjs @@ -27,8 +27,10 @@ const os = require('os') const path = require('path') const dns = require('dns').promises const net = require('net') +const { randomUUID } = require('crypto') const { pipeline } = require('stream/promises') const { Readable } = require('stream') +const { Agent } = require('undici') const EXTENSIONS = { cpuprofile: '.cpuprofile', @@ -300,7 +302,7 @@ async function validateConsoleUrl (consoleUrl) { } if (process.env.NSOLID_ALLOW_INSECURE_CONSOLE) { - return + return null } if (url.protocol !== 'https:') { @@ -322,6 +324,51 @@ async function validateConsoleUrl (consoleUrl) { throw new Error(`consoleUrl resolves to a private or local address: ${consoleUrl} (${ip})`) } } + + return ips +} + +function createPinnedDispatcher (consoleUrl, resolvedIps) { + const expectedHostname = new URL(consoleUrl).hostname.toLowerCase().replace(/^\[/, '').replace(/\]$/, '').replace(/\.$/, '') + const allowedIps = [...new Set(resolvedIps)] + + if (allowedIps.length === 0) { + throw new Error(`No validated addresses available for consoleUrl: ${consoleUrl}`) + } + + return new Agent({ + connect: { + autoSelectFamily: true, + lookup: (hostname, options, callback) => { + const requestedHostname = hostname.toLowerCase().replace(/\.$/, '') + if (requestedHostname !== expectedHostname) { + const error = new Error(`Refusing to resolve unvalidated hostname: ${hostname}`) + error.code = 'ENOTFOUND' + callback(error) + return + } + + const family = typeof options === 'number' ? options : options?.family + const matches = allowedIps + .map(address => ({ address, family: net.isIP(address) })) + .filter(record => record.family !== 0 && (!family || record.family === family)) + + if (matches.length === 0) { + const error = new Error(`No validated address matches the requested family for: ${hostname}`) + error.code = 'ENOTFOUND' + callback(error) + return + } + + if (typeof options === 'object' && options?.all) { + callback(null, matches) + return + } + + callback(null, matches[0].address, matches[0].family) + } + } + }) } async function readCredentials () { @@ -349,33 +396,55 @@ async function readCredentials () { throw new Error('Missing "serviceToken" in ~/.agents/.nodesource-auth.json') } - await validateConsoleUrl(consoleUrl) + const resolvedIps = await validateConsoleUrl(consoleUrl) - return { consoleUrl: consoleUrl.replace(/\/$/, ''), token } + return { consoleUrl: consoleUrl.replace(/\/$/, ''), token, resolvedIps } } -async function downloadAsset (consoleUrl, token, assetId, destPath) { +async function downloadAsset (consoleUrl, token, assetId, destPath, validatedIps) { const url = `${consoleUrl}/api/v3/asset/${encodeURIComponent(assetId)}` console.log(`Fetching asset from: ${url}`) - const res = await fetch(url, { - headers: { - 'x-nsolid-service-token': token, - Accept: 'application/json' - }, - // 10-minute total budget: large snapshots (>256MB) over slow links - // must not abort at 120s mid-body. - signal: AbortSignal.timeout(600_000) - }) + const resolvedIps = validatedIps === undefined + ? await validateConsoleUrl(consoleUrl) + : validatedIps + const dispatcher = resolvedIps === null + ? undefined + : createPinnedDispatcher(consoleUrl, resolvedIps) - if (!res.ok) { - throw new Error(`Console returned ${res.status} ${res.statusText} for asset ${assetId}`) - } + try { + const res = await fetch(url, { + headers: { + 'x-nsolid-service-token': token, + Accept: 'application/json' + }, + redirect: 'error', + dispatcher, + // 10-minute total budget: large snapshots (>256MB) over slow links + // must not abort at 120s mid-body. + signal: AbortSignal.timeout(600_000) + }) + + if (!res.ok) { + await res.body?.cancel() + throw new Error(`Console returned ${res.status} ${res.statusText} for asset ${assetId}`) + } + + // Keep incomplete bytes invisible to resolveExistingAsset(). The temporary + // file lives beside the destination so renameSync publishes it atomically. + const tempPath = `${destPath}.${process.pid}.${randomUUID()}.tmp` + try { + await pipeline(Readable.fromWeb(res.body), fs.createWriteStream(tempPath, { flags: 'wx' })) + fs.renameSync(tempPath, destPath) + } catch (error) { + fs.rmSync(tempPath, { force: true }) + throw error + } - // Stream body straight to disk — constant memory regardless of asset size. - // Node's fetch transparently decompresses Content-Encoding: gzip. - await pipeline(Readable.fromWeb(res.body), fs.createWriteStream(destPath)) - return fs.statSync(destPath).size + return fs.statSync(destPath).size + } finally { + await dispatcher?.close() + } } async function main () { @@ -394,7 +463,7 @@ async function main () { } const workspaceRoot = process.cwd() - const { consoleUrl, token } = await readCredentials() + const { consoleUrl, token, resolvedIps } = await readCredentials() const assetsDir = getAssetsDir(workspaceRoot) fs.mkdirSync(assetsDir, { recursive: true }) @@ -405,7 +474,7 @@ async function main () { if (existingAsset.exists) { fileSize = fs.statSync(existingAsset.filePath).size } else { - fileSize = await downloadAsset(consoleUrl, token, assetId, existingAsset.filePath) + fileSize = await downloadAsset(consoleUrl, token, assetId, existingAsset.filePath, resolvedIps) } // Register in .nsolid/assets/index.json so the extension's AssetService can discover it diff --git a/skills/ns-analyze-asset/fetch-asset.cjs b/skills/ns-analyze-asset/fetch-asset.cjs index 932dfe9..3f72628 100644 --- a/skills/ns-analyze-asset/fetch-asset.cjs +++ b/skills/ns-analyze-asset/fetch-asset.cjs @@ -27,8 +27,10 @@ const os = require('os') const path = require('path') const dns = require('dns').promises const net = require('net') +const { randomUUID } = require('crypto') const { pipeline } = require('stream/promises') const { Readable } = require('stream') +const { Agent } = require('undici') const EXTENSIONS = { cpuprofile: '.cpuprofile', @@ -300,7 +302,7 @@ async function validateConsoleUrl (consoleUrl) { } if (process.env.NSOLID_ALLOW_INSECURE_CONSOLE) { - return + return null } if (url.protocol !== 'https:') { @@ -322,6 +324,51 @@ async function validateConsoleUrl (consoleUrl) { throw new Error(`consoleUrl resolves to a private or local address: ${consoleUrl} (${ip})`) } } + + return ips +} + +function createPinnedDispatcher (consoleUrl, resolvedIps) { + const expectedHostname = new URL(consoleUrl).hostname.toLowerCase().replace(/^\[/, '').replace(/\]$/, '').replace(/\.$/, '') + const allowedIps = [...new Set(resolvedIps)] + + if (allowedIps.length === 0) { + throw new Error(`No validated addresses available for consoleUrl: ${consoleUrl}`) + } + + return new Agent({ + connect: { + autoSelectFamily: true, + lookup: (hostname, options, callback) => { + const requestedHostname = hostname.toLowerCase().replace(/\.$/, '') + if (requestedHostname !== expectedHostname) { + const error = new Error(`Refusing to resolve unvalidated hostname: ${hostname}`) + error.code = 'ENOTFOUND' + callback(error) + return + } + + const family = typeof options === 'number' ? options : options?.family + const matches = allowedIps + .map(address => ({ address, family: net.isIP(address) })) + .filter(record => record.family !== 0 && (!family || record.family === family)) + + if (matches.length === 0) { + const error = new Error(`No validated address matches the requested family for: ${hostname}`) + error.code = 'ENOTFOUND' + callback(error) + return + } + + if (typeof options === 'object' && options?.all) { + callback(null, matches) + return + } + + callback(null, matches[0].address, matches[0].family) + } + } + }) } async function readCredentials () { @@ -349,33 +396,55 @@ async function readCredentials () { throw new Error('Missing "serviceToken" in ~/.agents/.nodesource-auth.json') } - await validateConsoleUrl(consoleUrl) + const resolvedIps = await validateConsoleUrl(consoleUrl) - return { consoleUrl: consoleUrl.replace(/\/$/, ''), token } + return { consoleUrl: consoleUrl.replace(/\/$/, ''), token, resolvedIps } } -async function downloadAsset (consoleUrl, token, assetId, destPath) { +async function downloadAsset (consoleUrl, token, assetId, destPath, validatedIps) { const url = `${consoleUrl}/api/v3/asset/${encodeURIComponent(assetId)}` console.log(`Fetching asset from: ${url}`) - const res = await fetch(url, { - headers: { - 'x-nsolid-service-token': token, - Accept: 'application/json' - }, - // 10-minute total budget: large snapshots (>256MB) over slow links - // must not abort at 120s mid-body. - signal: AbortSignal.timeout(600_000) - }) + const resolvedIps = validatedIps === undefined + ? await validateConsoleUrl(consoleUrl) + : validatedIps + const dispatcher = resolvedIps === null + ? undefined + : createPinnedDispatcher(consoleUrl, resolvedIps) - if (!res.ok) { - throw new Error(`Console returned ${res.status} ${res.statusText} for asset ${assetId}`) - } + try { + const res = await fetch(url, { + headers: { + 'x-nsolid-service-token': token, + Accept: 'application/json' + }, + redirect: 'error', + dispatcher, + // 10-minute total budget: large snapshots (>256MB) over slow links + // must not abort at 120s mid-body. + signal: AbortSignal.timeout(600_000) + }) + + if (!res.ok) { + await res.body?.cancel() + throw new Error(`Console returned ${res.status} ${res.statusText} for asset ${assetId}`) + } + + // Keep incomplete bytes invisible to resolveExistingAsset(). The temporary + // file lives beside the destination so renameSync publishes it atomically. + const tempPath = `${destPath}.${process.pid}.${randomUUID()}.tmp` + try { + await pipeline(Readable.fromWeb(res.body), fs.createWriteStream(tempPath, { flags: 'wx' })) + fs.renameSync(tempPath, destPath) + } catch (error) { + fs.rmSync(tempPath, { force: true }) + throw error + } - // Stream body straight to disk — constant memory regardless of asset size. - // Node's fetch transparently decompresses Content-Encoding: gzip. - await pipeline(Readable.fromWeb(res.body), fs.createWriteStream(destPath)) - return fs.statSync(destPath).size + return fs.statSync(destPath).size + } finally { + await dispatcher?.close() + } } async function main () { @@ -394,7 +463,7 @@ async function main () { } const workspaceRoot = process.cwd() - const { consoleUrl, token } = await readCredentials() + const { consoleUrl, token, resolvedIps } = await readCredentials() const assetsDir = getAssetsDir(workspaceRoot) fs.mkdirSync(assetsDir, { recursive: true }) @@ -405,7 +474,7 @@ async function main () { if (existingAsset.exists) { fileSize = fs.statSync(existingAsset.filePath).size } else { - fileSize = await downloadAsset(consoleUrl, token, assetId, existingAsset.filePath) + fileSize = await downloadAsset(consoleUrl, token, assetId, existingAsset.filePath, resolvedIps) } // Register in .nsolid/assets/index.json so the extension's AssetService can discover it diff --git a/skills/ns-cpu-spike-analysis/fetch-asset.cjs b/skills/ns-cpu-spike-analysis/fetch-asset.cjs index 932dfe9..3f72628 100644 --- a/skills/ns-cpu-spike-analysis/fetch-asset.cjs +++ b/skills/ns-cpu-spike-analysis/fetch-asset.cjs @@ -27,8 +27,10 @@ const os = require('os') const path = require('path') const dns = require('dns').promises const net = require('net') +const { randomUUID } = require('crypto') const { pipeline } = require('stream/promises') const { Readable } = require('stream') +const { Agent } = require('undici') const EXTENSIONS = { cpuprofile: '.cpuprofile', @@ -300,7 +302,7 @@ async function validateConsoleUrl (consoleUrl) { } if (process.env.NSOLID_ALLOW_INSECURE_CONSOLE) { - return + return null } if (url.protocol !== 'https:') { @@ -322,6 +324,51 @@ async function validateConsoleUrl (consoleUrl) { throw new Error(`consoleUrl resolves to a private or local address: ${consoleUrl} (${ip})`) } } + + return ips +} + +function createPinnedDispatcher (consoleUrl, resolvedIps) { + const expectedHostname = new URL(consoleUrl).hostname.toLowerCase().replace(/^\[/, '').replace(/\]$/, '').replace(/\.$/, '') + const allowedIps = [...new Set(resolvedIps)] + + if (allowedIps.length === 0) { + throw new Error(`No validated addresses available for consoleUrl: ${consoleUrl}`) + } + + return new Agent({ + connect: { + autoSelectFamily: true, + lookup: (hostname, options, callback) => { + const requestedHostname = hostname.toLowerCase().replace(/\.$/, '') + if (requestedHostname !== expectedHostname) { + const error = new Error(`Refusing to resolve unvalidated hostname: ${hostname}`) + error.code = 'ENOTFOUND' + callback(error) + return + } + + const family = typeof options === 'number' ? options : options?.family + const matches = allowedIps + .map(address => ({ address, family: net.isIP(address) })) + .filter(record => record.family !== 0 && (!family || record.family === family)) + + if (matches.length === 0) { + const error = new Error(`No validated address matches the requested family for: ${hostname}`) + error.code = 'ENOTFOUND' + callback(error) + return + } + + if (typeof options === 'object' && options?.all) { + callback(null, matches) + return + } + + callback(null, matches[0].address, matches[0].family) + } + } + }) } async function readCredentials () { @@ -349,33 +396,55 @@ async function readCredentials () { throw new Error('Missing "serviceToken" in ~/.agents/.nodesource-auth.json') } - await validateConsoleUrl(consoleUrl) + const resolvedIps = await validateConsoleUrl(consoleUrl) - return { consoleUrl: consoleUrl.replace(/\/$/, ''), token } + return { consoleUrl: consoleUrl.replace(/\/$/, ''), token, resolvedIps } } -async function downloadAsset (consoleUrl, token, assetId, destPath) { +async function downloadAsset (consoleUrl, token, assetId, destPath, validatedIps) { const url = `${consoleUrl}/api/v3/asset/${encodeURIComponent(assetId)}` console.log(`Fetching asset from: ${url}`) - const res = await fetch(url, { - headers: { - 'x-nsolid-service-token': token, - Accept: 'application/json' - }, - // 10-minute total budget: large snapshots (>256MB) over slow links - // must not abort at 120s mid-body. - signal: AbortSignal.timeout(600_000) - }) + const resolvedIps = validatedIps === undefined + ? await validateConsoleUrl(consoleUrl) + : validatedIps + const dispatcher = resolvedIps === null + ? undefined + : createPinnedDispatcher(consoleUrl, resolvedIps) - if (!res.ok) { - throw new Error(`Console returned ${res.status} ${res.statusText} for asset ${assetId}`) - } + try { + const res = await fetch(url, { + headers: { + 'x-nsolid-service-token': token, + Accept: 'application/json' + }, + redirect: 'error', + dispatcher, + // 10-minute total budget: large snapshots (>256MB) over slow links + // must not abort at 120s mid-body. + signal: AbortSignal.timeout(600_000) + }) + + if (!res.ok) { + await res.body?.cancel() + throw new Error(`Console returned ${res.status} ${res.statusText} for asset ${assetId}`) + } + + // Keep incomplete bytes invisible to resolveExistingAsset(). The temporary + // file lives beside the destination so renameSync publishes it atomically. + const tempPath = `${destPath}.${process.pid}.${randomUUID()}.tmp` + try { + await pipeline(Readable.fromWeb(res.body), fs.createWriteStream(tempPath, { flags: 'wx' })) + fs.renameSync(tempPath, destPath) + } catch (error) { + fs.rmSync(tempPath, { force: true }) + throw error + } - // Stream body straight to disk — constant memory regardless of asset size. - // Node's fetch transparently decompresses Content-Encoding: gzip. - await pipeline(Readable.fromWeb(res.body), fs.createWriteStream(destPath)) - return fs.statSync(destPath).size + return fs.statSync(destPath).size + } finally { + await dispatcher?.close() + } } async function main () { @@ -394,7 +463,7 @@ async function main () { } const workspaceRoot = process.cwd() - const { consoleUrl, token } = await readCredentials() + const { consoleUrl, token, resolvedIps } = await readCredentials() const assetsDir = getAssetsDir(workspaceRoot) fs.mkdirSync(assetsDir, { recursive: true }) @@ -405,7 +474,7 @@ async function main () { if (existingAsset.exists) { fileSize = fs.statSync(existingAsset.filePath).size } else { - fileSize = await downloadAsset(consoleUrl, token, assetId, existingAsset.filePath) + fileSize = await downloadAsset(consoleUrl, token, assetId, existingAsset.filePath, resolvedIps) } // Register in .nsolid/assets/index.json so the extension's AssetService can discover it diff --git a/skills/ns-download-asset/SKILL.md b/skills/ns-download-asset/SKILL.md index 860369b..f022090 100644 --- a/skills/ns-download-asset/SKILL.md +++ b/skills/ns-download-asset/SKILL.md @@ -26,7 +26,7 @@ description: >- Run the bundled script (use the absolute path of the directory where you read this SKILL.md): -``` +```sh node "/fetch-asset.cjs" ``` diff --git a/skills/ns-download-asset/fetch-asset.cjs b/skills/ns-download-asset/fetch-asset.cjs index 932dfe9..3f72628 100644 --- a/skills/ns-download-asset/fetch-asset.cjs +++ b/skills/ns-download-asset/fetch-asset.cjs @@ -27,8 +27,10 @@ const os = require('os') const path = require('path') const dns = require('dns').promises const net = require('net') +const { randomUUID } = require('crypto') const { pipeline } = require('stream/promises') const { Readable } = require('stream') +const { Agent } = require('undici') const EXTENSIONS = { cpuprofile: '.cpuprofile', @@ -300,7 +302,7 @@ async function validateConsoleUrl (consoleUrl) { } if (process.env.NSOLID_ALLOW_INSECURE_CONSOLE) { - return + return null } if (url.protocol !== 'https:') { @@ -322,6 +324,51 @@ async function validateConsoleUrl (consoleUrl) { throw new Error(`consoleUrl resolves to a private or local address: ${consoleUrl} (${ip})`) } } + + return ips +} + +function createPinnedDispatcher (consoleUrl, resolvedIps) { + const expectedHostname = new URL(consoleUrl).hostname.toLowerCase().replace(/^\[/, '').replace(/\]$/, '').replace(/\.$/, '') + const allowedIps = [...new Set(resolvedIps)] + + if (allowedIps.length === 0) { + throw new Error(`No validated addresses available for consoleUrl: ${consoleUrl}`) + } + + return new Agent({ + connect: { + autoSelectFamily: true, + lookup: (hostname, options, callback) => { + const requestedHostname = hostname.toLowerCase().replace(/\.$/, '') + if (requestedHostname !== expectedHostname) { + const error = new Error(`Refusing to resolve unvalidated hostname: ${hostname}`) + error.code = 'ENOTFOUND' + callback(error) + return + } + + const family = typeof options === 'number' ? options : options?.family + const matches = allowedIps + .map(address => ({ address, family: net.isIP(address) })) + .filter(record => record.family !== 0 && (!family || record.family === family)) + + if (matches.length === 0) { + const error = new Error(`No validated address matches the requested family for: ${hostname}`) + error.code = 'ENOTFOUND' + callback(error) + return + } + + if (typeof options === 'object' && options?.all) { + callback(null, matches) + return + } + + callback(null, matches[0].address, matches[0].family) + } + } + }) } async function readCredentials () { @@ -349,33 +396,55 @@ async function readCredentials () { throw new Error('Missing "serviceToken" in ~/.agents/.nodesource-auth.json') } - await validateConsoleUrl(consoleUrl) + const resolvedIps = await validateConsoleUrl(consoleUrl) - return { consoleUrl: consoleUrl.replace(/\/$/, ''), token } + return { consoleUrl: consoleUrl.replace(/\/$/, ''), token, resolvedIps } } -async function downloadAsset (consoleUrl, token, assetId, destPath) { +async function downloadAsset (consoleUrl, token, assetId, destPath, validatedIps) { const url = `${consoleUrl}/api/v3/asset/${encodeURIComponent(assetId)}` console.log(`Fetching asset from: ${url}`) - const res = await fetch(url, { - headers: { - 'x-nsolid-service-token': token, - Accept: 'application/json' - }, - // 10-minute total budget: large snapshots (>256MB) over slow links - // must not abort at 120s mid-body. - signal: AbortSignal.timeout(600_000) - }) + const resolvedIps = validatedIps === undefined + ? await validateConsoleUrl(consoleUrl) + : validatedIps + const dispatcher = resolvedIps === null + ? undefined + : createPinnedDispatcher(consoleUrl, resolvedIps) - if (!res.ok) { - throw new Error(`Console returned ${res.status} ${res.statusText} for asset ${assetId}`) - } + try { + const res = await fetch(url, { + headers: { + 'x-nsolid-service-token': token, + Accept: 'application/json' + }, + redirect: 'error', + dispatcher, + // 10-minute total budget: large snapshots (>256MB) over slow links + // must not abort at 120s mid-body. + signal: AbortSignal.timeout(600_000) + }) + + if (!res.ok) { + await res.body?.cancel() + throw new Error(`Console returned ${res.status} ${res.statusText} for asset ${assetId}`) + } + + // Keep incomplete bytes invisible to resolveExistingAsset(). The temporary + // file lives beside the destination so renameSync publishes it atomically. + const tempPath = `${destPath}.${process.pid}.${randomUUID()}.tmp` + try { + await pipeline(Readable.fromWeb(res.body), fs.createWriteStream(tempPath, { flags: 'wx' })) + fs.renameSync(tempPath, destPath) + } catch (error) { + fs.rmSync(tempPath, { force: true }) + throw error + } - // Stream body straight to disk — constant memory regardless of asset size. - // Node's fetch transparently decompresses Content-Encoding: gzip. - await pipeline(Readable.fromWeb(res.body), fs.createWriteStream(destPath)) - return fs.statSync(destPath).size + return fs.statSync(destPath).size + } finally { + await dispatcher?.close() + } } async function main () { @@ -394,7 +463,7 @@ async function main () { } const workspaceRoot = process.cwd() - const { consoleUrl, token } = await readCredentials() + const { consoleUrl, token, resolvedIps } = await readCredentials() const assetsDir = getAssetsDir(workspaceRoot) fs.mkdirSync(assetsDir, { recursive: true }) @@ -405,7 +474,7 @@ async function main () { if (existingAsset.exists) { fileSize = fs.statSync(existingAsset.filePath).size } else { - fileSize = await downloadAsset(consoleUrl, token, assetId, existingAsset.filePath) + fileSize = await downloadAsset(consoleUrl, token, assetId, existingAsset.filePath, resolvedIps) } // Register in .nsolid/assets/index.json so the extension's AssetService can discover it diff --git a/skills/ns-generate-asset/fetch-asset.cjs b/skills/ns-generate-asset/fetch-asset.cjs index 932dfe9..3f72628 100644 --- a/skills/ns-generate-asset/fetch-asset.cjs +++ b/skills/ns-generate-asset/fetch-asset.cjs @@ -27,8 +27,10 @@ const os = require('os') const path = require('path') const dns = require('dns').promises const net = require('net') +const { randomUUID } = require('crypto') const { pipeline } = require('stream/promises') const { Readable } = require('stream') +const { Agent } = require('undici') const EXTENSIONS = { cpuprofile: '.cpuprofile', @@ -300,7 +302,7 @@ async function validateConsoleUrl (consoleUrl) { } if (process.env.NSOLID_ALLOW_INSECURE_CONSOLE) { - return + return null } if (url.protocol !== 'https:') { @@ -322,6 +324,51 @@ async function validateConsoleUrl (consoleUrl) { throw new Error(`consoleUrl resolves to a private or local address: ${consoleUrl} (${ip})`) } } + + return ips +} + +function createPinnedDispatcher (consoleUrl, resolvedIps) { + const expectedHostname = new URL(consoleUrl).hostname.toLowerCase().replace(/^\[/, '').replace(/\]$/, '').replace(/\.$/, '') + const allowedIps = [...new Set(resolvedIps)] + + if (allowedIps.length === 0) { + throw new Error(`No validated addresses available for consoleUrl: ${consoleUrl}`) + } + + return new Agent({ + connect: { + autoSelectFamily: true, + lookup: (hostname, options, callback) => { + const requestedHostname = hostname.toLowerCase().replace(/\.$/, '') + if (requestedHostname !== expectedHostname) { + const error = new Error(`Refusing to resolve unvalidated hostname: ${hostname}`) + error.code = 'ENOTFOUND' + callback(error) + return + } + + const family = typeof options === 'number' ? options : options?.family + const matches = allowedIps + .map(address => ({ address, family: net.isIP(address) })) + .filter(record => record.family !== 0 && (!family || record.family === family)) + + if (matches.length === 0) { + const error = new Error(`No validated address matches the requested family for: ${hostname}`) + error.code = 'ENOTFOUND' + callback(error) + return + } + + if (typeof options === 'object' && options?.all) { + callback(null, matches) + return + } + + callback(null, matches[0].address, matches[0].family) + } + } + }) } async function readCredentials () { @@ -349,33 +396,55 @@ async function readCredentials () { throw new Error('Missing "serviceToken" in ~/.agents/.nodesource-auth.json') } - await validateConsoleUrl(consoleUrl) + const resolvedIps = await validateConsoleUrl(consoleUrl) - return { consoleUrl: consoleUrl.replace(/\/$/, ''), token } + return { consoleUrl: consoleUrl.replace(/\/$/, ''), token, resolvedIps } } -async function downloadAsset (consoleUrl, token, assetId, destPath) { +async function downloadAsset (consoleUrl, token, assetId, destPath, validatedIps) { const url = `${consoleUrl}/api/v3/asset/${encodeURIComponent(assetId)}` console.log(`Fetching asset from: ${url}`) - const res = await fetch(url, { - headers: { - 'x-nsolid-service-token': token, - Accept: 'application/json' - }, - // 10-minute total budget: large snapshots (>256MB) over slow links - // must not abort at 120s mid-body. - signal: AbortSignal.timeout(600_000) - }) + const resolvedIps = validatedIps === undefined + ? await validateConsoleUrl(consoleUrl) + : validatedIps + const dispatcher = resolvedIps === null + ? undefined + : createPinnedDispatcher(consoleUrl, resolvedIps) - if (!res.ok) { - throw new Error(`Console returned ${res.status} ${res.statusText} for asset ${assetId}`) - } + try { + const res = await fetch(url, { + headers: { + 'x-nsolid-service-token': token, + Accept: 'application/json' + }, + redirect: 'error', + dispatcher, + // 10-minute total budget: large snapshots (>256MB) over slow links + // must not abort at 120s mid-body. + signal: AbortSignal.timeout(600_000) + }) + + if (!res.ok) { + await res.body?.cancel() + throw new Error(`Console returned ${res.status} ${res.statusText} for asset ${assetId}`) + } + + // Keep incomplete bytes invisible to resolveExistingAsset(). The temporary + // file lives beside the destination so renameSync publishes it atomically. + const tempPath = `${destPath}.${process.pid}.${randomUUID()}.tmp` + try { + await pipeline(Readable.fromWeb(res.body), fs.createWriteStream(tempPath, { flags: 'wx' })) + fs.renameSync(tempPath, destPath) + } catch (error) { + fs.rmSync(tempPath, { force: true }) + throw error + } - // Stream body straight to disk — constant memory regardless of asset size. - // Node's fetch transparently decompresses Content-Encoding: gzip. - await pipeline(Readable.fromWeb(res.body), fs.createWriteStream(destPath)) - return fs.statSync(destPath).size + return fs.statSync(destPath).size + } finally { + await dispatcher?.close() + } } async function main () { @@ -394,7 +463,7 @@ async function main () { } const workspaceRoot = process.cwd() - const { consoleUrl, token } = await readCredentials() + const { consoleUrl, token, resolvedIps } = await readCredentials() const assetsDir = getAssetsDir(workspaceRoot) fs.mkdirSync(assetsDir, { recursive: true }) @@ -405,7 +474,7 @@ async function main () { if (existingAsset.exists) { fileSize = fs.statSync(existingAsset.filePath).size } else { - fileSize = await downloadAsset(consoleUrl, token, assetId, existingAsset.filePath) + fileSize = await downloadAsset(consoleUrl, token, assetId, existingAsset.filePath, resolvedIps) } // Register in .nsolid/assets/index.json so the extension's AssetService can discover it diff --git a/skills/ns-memory-spike-analysis/fetch-asset.cjs b/skills/ns-memory-spike-analysis/fetch-asset.cjs index 932dfe9..3f72628 100644 --- a/skills/ns-memory-spike-analysis/fetch-asset.cjs +++ b/skills/ns-memory-spike-analysis/fetch-asset.cjs @@ -27,8 +27,10 @@ const os = require('os') const path = require('path') const dns = require('dns').promises const net = require('net') +const { randomUUID } = require('crypto') const { pipeline } = require('stream/promises') const { Readable } = require('stream') +const { Agent } = require('undici') const EXTENSIONS = { cpuprofile: '.cpuprofile', @@ -300,7 +302,7 @@ async function validateConsoleUrl (consoleUrl) { } if (process.env.NSOLID_ALLOW_INSECURE_CONSOLE) { - return + return null } if (url.protocol !== 'https:') { @@ -322,6 +324,51 @@ async function validateConsoleUrl (consoleUrl) { throw new Error(`consoleUrl resolves to a private or local address: ${consoleUrl} (${ip})`) } } + + return ips +} + +function createPinnedDispatcher (consoleUrl, resolvedIps) { + const expectedHostname = new URL(consoleUrl).hostname.toLowerCase().replace(/^\[/, '').replace(/\]$/, '').replace(/\.$/, '') + const allowedIps = [...new Set(resolvedIps)] + + if (allowedIps.length === 0) { + throw new Error(`No validated addresses available for consoleUrl: ${consoleUrl}`) + } + + return new Agent({ + connect: { + autoSelectFamily: true, + lookup: (hostname, options, callback) => { + const requestedHostname = hostname.toLowerCase().replace(/\.$/, '') + if (requestedHostname !== expectedHostname) { + const error = new Error(`Refusing to resolve unvalidated hostname: ${hostname}`) + error.code = 'ENOTFOUND' + callback(error) + return + } + + const family = typeof options === 'number' ? options : options?.family + const matches = allowedIps + .map(address => ({ address, family: net.isIP(address) })) + .filter(record => record.family !== 0 && (!family || record.family === family)) + + if (matches.length === 0) { + const error = new Error(`No validated address matches the requested family for: ${hostname}`) + error.code = 'ENOTFOUND' + callback(error) + return + } + + if (typeof options === 'object' && options?.all) { + callback(null, matches) + return + } + + callback(null, matches[0].address, matches[0].family) + } + } + }) } async function readCredentials () { @@ -349,33 +396,55 @@ async function readCredentials () { throw new Error('Missing "serviceToken" in ~/.agents/.nodesource-auth.json') } - await validateConsoleUrl(consoleUrl) + const resolvedIps = await validateConsoleUrl(consoleUrl) - return { consoleUrl: consoleUrl.replace(/\/$/, ''), token } + return { consoleUrl: consoleUrl.replace(/\/$/, ''), token, resolvedIps } } -async function downloadAsset (consoleUrl, token, assetId, destPath) { +async function downloadAsset (consoleUrl, token, assetId, destPath, validatedIps) { const url = `${consoleUrl}/api/v3/asset/${encodeURIComponent(assetId)}` console.log(`Fetching asset from: ${url}`) - const res = await fetch(url, { - headers: { - 'x-nsolid-service-token': token, - Accept: 'application/json' - }, - // 10-minute total budget: large snapshots (>256MB) over slow links - // must not abort at 120s mid-body. - signal: AbortSignal.timeout(600_000) - }) + const resolvedIps = validatedIps === undefined + ? await validateConsoleUrl(consoleUrl) + : validatedIps + const dispatcher = resolvedIps === null + ? undefined + : createPinnedDispatcher(consoleUrl, resolvedIps) - if (!res.ok) { - throw new Error(`Console returned ${res.status} ${res.statusText} for asset ${assetId}`) - } + try { + const res = await fetch(url, { + headers: { + 'x-nsolid-service-token': token, + Accept: 'application/json' + }, + redirect: 'error', + dispatcher, + // 10-minute total budget: large snapshots (>256MB) over slow links + // must not abort at 120s mid-body. + signal: AbortSignal.timeout(600_000) + }) + + if (!res.ok) { + await res.body?.cancel() + throw new Error(`Console returned ${res.status} ${res.statusText} for asset ${assetId}`) + } + + // Keep incomplete bytes invisible to resolveExistingAsset(). The temporary + // file lives beside the destination so renameSync publishes it atomically. + const tempPath = `${destPath}.${process.pid}.${randomUUID()}.tmp` + try { + await pipeline(Readable.fromWeb(res.body), fs.createWriteStream(tempPath, { flags: 'wx' })) + fs.renameSync(tempPath, destPath) + } catch (error) { + fs.rmSync(tempPath, { force: true }) + throw error + } - // Stream body straight to disk — constant memory regardless of asset size. - // Node's fetch transparently decompresses Content-Encoding: gzip. - await pipeline(Readable.fromWeb(res.body), fs.createWriteStream(destPath)) - return fs.statSync(destPath).size + return fs.statSync(destPath).size + } finally { + await dispatcher?.close() + } } async function main () { @@ -394,7 +463,7 @@ async function main () { } const workspaceRoot = process.cwd() - const { consoleUrl, token } = await readCredentials() + const { consoleUrl, token, resolvedIps } = await readCredentials() const assetsDir = getAssetsDir(workspaceRoot) fs.mkdirSync(assetsDir, { recursive: true }) @@ -405,7 +474,7 @@ async function main () { if (existingAsset.exists) { fileSize = fs.statSync(existingAsset.filePath).size } else { - fileSize = await downloadAsset(consoleUrl, token, assetId, existingAsset.filePath) + fileSize = await downloadAsset(consoleUrl, token, assetId, existingAsset.filePath, resolvedIps) } // Register in .nsolid/assets/index.json so the extension's AssetService can discover it From f24de1f833e7293b4c886207846d27c3b6b2fded Mon Sep 17 00:00:00 2001 From: Cesar-M-Diaz Date: Mon, 24 Aug 2026 15:10:20 +0200 Subject: [PATCH 3/8] fix(skills): drop undici; keep fetch-asset.cjs standalone Replace the undici Agent dispatcher with a dns.lookup-compatible pinned lookup passed to http/https.request, closing the same DNS-rebinding gap without any runtime dependency. Native plugin installs do not install root devDependencies, so require('undici') broke the standalone script with MODULE_NOT_FOUND. Redirects are never followed by http/https.request, so the service token cannot leak to another origin (previously redirect: 'error'). Removes undici from root devDependencies and packages/core dependencies; the lockfile only keeps it as a transitive dep of mcp-remote. --- package.json | 3 +- packages/core/package.json | 1 - .../core/test/unit/skills/fetch-asset.test.ts | 112 ++++++++++---- pnpm-lock.yaml | 6 - skill-assets/fetch-asset.cjs | 141 ++++++++++-------- .../fetch-asset.cjs | 141 ++++++++++-------- skills/ns-analyze-asset/fetch-asset.cjs | 141 ++++++++++-------- skills/ns-cpu-spike-analysis/fetch-asset.cjs | 141 ++++++++++-------- skills/ns-download-asset/fetch-asset.cjs | 141 ++++++++++-------- skills/ns-generate-asset/fetch-asset.cjs | 141 ++++++++++-------- .../ns-memory-spike-analysis/fetch-asset.cjs | 141 ++++++++++-------- 11 files changed, 635 insertions(+), 474 deletions(-) diff --git a/package.json b/package.json index 6aeb697..69fb67e 100644 --- a/package.json +++ b/package.json @@ -30,8 +30,7 @@ "eslint": "9.39.4", "husky": "9.1.7", "neostandard": "0.13.0", - "tsx": "4.22.4", - "undici": "7.28.0" + "tsx": "4.22.4" }, "dependencies": { "mcp-remote": "0.1.38" diff --git a/packages/core/package.json b/packages/core/package.json index 87c4a78..81fc2b8 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -31,7 +31,6 @@ }, "dependencies": { "smol-toml": "^1.3.1", - "undici": "7.28.0", "write-file-atomic": "^5.0.1", "zod": "4.4.3" }, diff --git a/packages/core/test/unit/skills/fetch-asset.test.ts b/packages/core/test/unit/skills/fetch-asset.test.ts index b3f3173..1b5a605 100644 --- a/packages/core/test/unit/skills/fetch-asset.test.ts +++ b/packages/core/test/unit/skills/fetch-asset.test.ts @@ -5,7 +5,11 @@ import { fileURLToPath, pathToFileURL } from 'node:url' import { dirname, join } from 'node:path' import { mkdtemp, readFile, readdir, rm, stat } from 'node:fs/promises' import { tmpdir } from 'node:os' +import https from 'node:https' +import { EventEmitter } from 'node:events' +import { Readable } from 'node:stream' import type { TestContext } from 'node:test' +import type { ClientRequest, IncomingMessage, RequestOptions } from 'node:http' const __dirname = dirname(fileURLToPath(import.meta.url)) const fetchAssetPath = join(__dirname, '../../../../../skill-assets/fetch-asset.cjs') @@ -251,6 +255,43 @@ describe('validateConsoleUrl', () => { }) }) +function makeFakeRequest (respond: () => IncomingMessage) { + const req = new EventEmitter() as ClientRequest + req.setTimeout = () => req + req.end = (() => { + respond() + return req + }) as ClientRequest['end'] + req.destroy = ((error?: Error) => { + if (error) queueMicrotask(() => req.emit('error', error)) + return req + }) as ClientRequest['destroy'] + return req +} + +function makeFakeResponse (body: Buffer | ((this: Readable) => void), statusCode = 200, statusMessage = 'OK') { + let readCount = 0 + const res = new Readable({ + read () { + if (readCount++ === 0) { + if (Buffer.isBuffer(body)) { + this.push(body) + } else { + // Function bodies own the stream lifecycle (EOF and/or error). + body.call(this) + } + return + } + if (Buffer.isBuffer(body)) { + this.push(null) + } + } + }) as IncomingMessage + res.statusCode = statusCode + res.statusMessage = statusMessage + return res +} + describe('downloadAsset', () => { let tempDir: string @@ -263,23 +304,20 @@ describe('downloadAsset', () => { }) it('streams the asset body to disk and returns the file size', async (t) => { - const payload = new Uint8Array([0, 1, 2, 3, 4, 5, 6, 7, 8, 9]) + const payload = Buffer.from([0, 1, 2, 3, 4, 5, 6, 7, 8, 9]) const calls: Array<{ url: string - headers: Record - redirect: RequestRedirect | undefined - dispatcher: unknown + headers: Record + lookup: unknown }> = [] - t.mock.method(globalThis, 'fetch', (async (input: RequestInfo | URL, init?: RequestInit) => { - const options = init as RequestInit & { dispatcher?: unknown } + t.mock.method(https, 'request', ((input: string | URL, options: RequestOptions, callback?: (res: IncomingMessage) => void) => { calls.push({ url: String(input), - headers: (init?.headers ?? {}) as Record, - redirect: init?.redirect, - dispatcher: options.dispatcher, + headers: (options.headers ?? {}) as Record, + lookup: options.lookup }) - return new Response(payload) - }) as typeof fetch) + return makeFakeRequest(() => callback?.(makeFakeResponse(payload))) + }) as typeof https.request) const { downloadAsset } = await loadFetchAsset() const destPath = join(tempDir, 'heapsnapshot-test.heapsnapshot') @@ -295,22 +333,21 @@ describe('downloadAsset', () => { const stats = await stat(destPath) assert.equal(size, stats.size, 'returns the on-disk file size') assert.equal(size, payload.byteLength) - assert.deepEqual(await readFile(destPath), Buffer.from(payload), 'file bytes match the response body') + assert.deepEqual(await readFile(destPath), payload, 'file bytes match the response body') assert.equal(calls.length, 1) assert.equal(calls[0].url, 'https://console.example.test/api/v3/asset/asset-id-1') assert.equal(calls[0].headers['x-nsolid-service-token'], 'secret-token') assert.equal(calls[0].headers.Accept, 'application/json') - assert.equal(calls[0].redirect, 'error') - assert.ok(calls[0].dispatcher, 'uses a dispatcher pinned to validated addresses') + assert.equal(typeof calls[0].lookup, 'function', 'uses a lookup pinned to validated addresses') assert.deepEqual(await readdir(tempDir), ['heapsnapshot-test.heapsnapshot']) }) it('URL-encodes the asset ID in the request path', async (t) => { const urls: string[] = [] - t.mock.method(globalThis, 'fetch', (async (input: RequestInfo | URL) => { + t.mock.method(https, 'request', ((input: string | URL, _options: RequestOptions, callback?: (res: IncomingMessage) => void) => { urls.push(String(input)) - return new Response(new Uint8Array([1, 2, 3])) - }) as typeof fetch) + return makeFakeRequest(() => callback?.(makeFakeResponse(Buffer.from([1, 2, 3])))) + }) as typeof https.request) const { downloadAsset } = await loadFetchAsset() await downloadAsset( @@ -325,9 +362,9 @@ describe('downloadAsset', () => { }) it('throws on non-ok responses without creating a file', async (t) => { - t.mock.method(globalThis, 'fetch', (async () => { - return new Response('not found', { status: 404, statusText: 'Not Found' }) - }) as typeof fetch) + t.mock.method(https, 'request', ((_input: string | URL, _options: RequestOptions, callback?: (res: IncomingMessage) => void) => { + return makeFakeRequest(() => callback?.(makeFakeResponse(Buffer.from('not found'), 404, 'Not Found'))) + }) as typeof https.request) const { downloadAsset } = await loadFetchAsset() const destPath = join(tempDir, 'missing.heapsnapshot') @@ -339,17 +376,12 @@ describe('downloadAsset', () => { }) it('removes temporary bytes when the response stream fails', async (t) => { - let pullCount = 0 - const body = new ReadableStream({ - pull (controller) { - if (pullCount++ === 0) { - controller.enqueue(new Uint8Array([1, 2, 3])) - return - } - controller.error(new Error('stream interrupted')) - }, - }) - t.mock.method(globalThis, 'fetch', (async () => new Response(body)) as typeof fetch) + t.mock.method(https, 'request', ((_input: string | URL, _options: RequestOptions, callback?: (res: IncomingMessage) => void) => { + return makeFakeRequest(() => callback?.(makeFakeResponse(function (this: Readable) { + this.push(Buffer.from([1, 2, 3])) + queueMicrotask(() => this.destroy(new Error('stream interrupted'))) + }))) + }) as typeof https.request) const { downloadAsset } = await loadFetchAsset() const destPath = join(tempDir, 'partial.heapsnapshot') @@ -361,4 +393,22 @@ describe('downloadAsset', () => { await assert.rejects(() => stat(destPath), /ENOENT/) assert.deepEqual(await readdir(tempDir), [], 'does not leave a temporary file behind') }) + + it('aborts when the whole exchange exceeds the 10-minute deadline', async (t) => { + t.mock.timers.enable({ apis: ['setTimeout'] }) + // The request never responds: covers connect/headers stalling past the + // absolute deadline (req.setTimeout would not catch this — it only + // measures socket inactivity, and the body timer only starts after + // headers arrive). + t.mock.method(https, 'request', (() => makeFakeRequest(() => {})) as typeof https.request) + + const { downloadAsset } = await loadFetchAsset() + const destPath = join(tempDir, 'stalled.heapsnapshot') + const pending = downloadAsset('https://console.example.test', 'token', 'stalled-id', destPath, ['93.184.216.34']) + const assertion = assert.rejects(pending, /exceeded 10 minutes/) + t.mock.timers.tick(600_000) + await assertion + await assert.rejects(() => stat(destPath), /ENOENT/) + assert.deepEqual(await readdir(tempDir), [], 'does not leave a temporary file behind') + }) }) diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 457c831..d9c3a01 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -24,18 +24,12 @@ importers: tsx: specifier: 4.22.4 version: 4.22.4 - undici: - specifier: 7.28.0 - version: 7.28.0 packages/core: dependencies: smol-toml: specifier: ^1.3.1 version: 1.6.1 - undici: - specifier: 7.28.0 - version: 7.28.0 write-file-atomic: specifier: ^5.0.1 version: 5.0.1 diff --git a/skill-assets/fetch-asset.cjs b/skill-assets/fetch-asset.cjs index 3f72628..b3090f2 100644 --- a/skill-assets/fetch-asset.cjs +++ b/skill-assets/fetch-asset.cjs @@ -29,8 +29,8 @@ const dns = require('dns').promises const net = require('net') const { randomUUID } = require('crypto') const { pipeline } = require('stream/promises') -const { Readable } = require('stream') -const { Agent } = require('undici') +const http = require('http') +const https = require('https') const EXTENSIONS = { cpuprofile: '.cpuprofile', @@ -328,7 +328,12 @@ async function validateConsoleUrl (consoleUrl) { return ips } -function createPinnedDispatcher (consoleUrl, resolvedIps) { +// Returns a dns.lookup-compatible function that resolves the console hostname +// exclusively to the addresses validated by validateConsoleUrl(). Passing it as +// the `lookup` option of http/https.request pins the connection to those +// addresses and closes the DNS-rebinding gap between validation and connect. +// Built-in modules only — this script must stay standalone (no node_modules). +function createPinnedLookup (consoleUrl, resolvedIps) { const expectedHostname = new URL(consoleUrl).hostname.toLowerCase().replace(/^\[/, '').replace(/\]$/, '').replace(/\.$/, '') const allowedIps = [...new Set(resolvedIps)] @@ -336,39 +341,34 @@ function createPinnedDispatcher (consoleUrl, resolvedIps) { throw new Error(`No validated addresses available for consoleUrl: ${consoleUrl}`) } - return new Agent({ - connect: { - autoSelectFamily: true, - lookup: (hostname, options, callback) => { - const requestedHostname = hostname.toLowerCase().replace(/\.$/, '') - if (requestedHostname !== expectedHostname) { - const error = new Error(`Refusing to resolve unvalidated hostname: ${hostname}`) - error.code = 'ENOTFOUND' - callback(error) - return - } - - const family = typeof options === 'number' ? options : options?.family - const matches = allowedIps - .map(address => ({ address, family: net.isIP(address) })) - .filter(record => record.family !== 0 && (!family || record.family === family)) - - if (matches.length === 0) { - const error = new Error(`No validated address matches the requested family for: ${hostname}`) - error.code = 'ENOTFOUND' - callback(error) - return - } - - if (typeof options === 'object' && options?.all) { - callback(null, matches) - return - } - - callback(null, matches[0].address, matches[0].family) - } + return (hostname, options, callback) => { + const requestedHostname = hostname.toLowerCase().replace(/\.$/, '') + if (requestedHostname !== expectedHostname) { + const error = new Error(`Refusing to resolve unvalidated hostname: ${hostname}`) + error.code = 'ENOTFOUND' + callback(error) + return } - }) + + const family = typeof options === 'number' ? options : options?.family + const matches = allowedIps + .map(address => ({ address, family: net.isIP(address) })) + .filter(record => record.family !== 0 && (!family || record.family === family)) + + if (matches.length === 0) { + const error = new Error(`No validated address matches the requested family for: ${hostname}`) + error.code = 'ENOTFOUND' + callback(error) + return + } + + if (typeof options === 'object' && options?.all) { + callback(null, matches) + return + } + + callback(null, matches[0].address, matches[0].family) + } } async function readCredentials () { @@ -402,48 +402,65 @@ async function readCredentials () { } async function downloadAsset (consoleUrl, token, assetId, destPath, validatedIps) { - const url = `${consoleUrl}/api/v3/asset/${encodeURIComponent(assetId)}` + const url = new URL(`${consoleUrl}/api/v3/asset/${encodeURIComponent(assetId)}`) console.log(`Fetching asset from: ${url}`) const resolvedIps = validatedIps === undefined ? await validateConsoleUrl(consoleUrl) : validatedIps - const dispatcher = resolvedIps === null + const lookup = resolvedIps === null ? undefined - : createPinnedDispatcher(consoleUrl, resolvedIps) + : createPinnedLookup(consoleUrl, resolvedIps) + + // Keep incomplete bytes invisible to resolveExistingAsset(). The temporary + // file lives beside the destination so renameSync publishes it atomically. + const tempPath = `${destPath}.${process.pid}.${randomUUID()}.tmp` + + // One absolute deadline spanning connect, response headers, and body — the + // same total budget AbortSignal.timeout(600_000) enforced before. + // req.setTimeout() would only measure socket inactivity, so a stalled + // connection could otherwise exceed the budget indefinitely. + let abortInFlight = () => {} + const deadline = setTimeout(() => { + abortInFlight(new Error(`Download of asset ${assetId} exceeded 10 minutes`)) + }, 600_000) try { - const res = await fetch(url, { - headers: { - 'x-nsolid-service-token': token, - Accept: 'application/json' - }, - redirect: 'error', - dispatcher, - // 10-minute total budget: large snapshots (>256MB) over slow links - // must not abort at 120s mid-body. - signal: AbortSignal.timeout(600_000) + const res = await new Promise((resolve, reject) => { + // http/https.request never follows redirects, so the service token can + // never be forwarded to a different origin. + const transport = url.protocol === 'http:' ? http : https + const req = transport.request(url, { + headers: { + 'x-nsolid-service-token': token, + Accept: 'application/json' + }, + lookup + }, resolve) + abortInFlight = (error) => req.destroy(error) + req.on('error', reject) + req.end() }) - if (!res.ok) { - await res.body?.cancel() - throw new Error(`Console returned ${res.status} ${res.statusText} for asset ${assetId}`) + if (res.statusCode < 200 || res.statusCode >= 300) { + res.resume() // drain the socket before throwing + const isRedirect = res.statusCode >= 300 && res.statusCode < 400 + throw new Error( + `Console returned ${res.statusCode} ${res.statusMessage} for asset ${assetId}` + + (isRedirect ? ' (redirects are not followed)' : '') + ) } - // Keep incomplete bytes invisible to resolveExistingAsset(). The temporary - // file lives beside the destination so renameSync publishes it atomically. - const tempPath = `${destPath}.${process.pid}.${randomUUID()}.tmp` - try { - await pipeline(Readable.fromWeb(res.body), fs.createWriteStream(tempPath, { flags: 'wx' })) - fs.renameSync(tempPath, destPath) - } catch (error) { - fs.rmSync(tempPath, { force: true }) - throw error - } + abortInFlight = (error) => res.destroy(error) + await pipeline(res, fs.createWriteStream(tempPath, { flags: 'wx' })) + fs.renameSync(tempPath, destPath) return fs.statSync(destPath).size + } catch (error) { + fs.rmSync(tempPath, { force: true }) + throw error } finally { - await dispatcher?.close() + clearTimeout(deadline) } } diff --git a/skills/ns-advanced-memory-leak-hunter/fetch-asset.cjs b/skills/ns-advanced-memory-leak-hunter/fetch-asset.cjs index 3f72628..b3090f2 100644 --- a/skills/ns-advanced-memory-leak-hunter/fetch-asset.cjs +++ b/skills/ns-advanced-memory-leak-hunter/fetch-asset.cjs @@ -29,8 +29,8 @@ const dns = require('dns').promises const net = require('net') const { randomUUID } = require('crypto') const { pipeline } = require('stream/promises') -const { Readable } = require('stream') -const { Agent } = require('undici') +const http = require('http') +const https = require('https') const EXTENSIONS = { cpuprofile: '.cpuprofile', @@ -328,7 +328,12 @@ async function validateConsoleUrl (consoleUrl) { return ips } -function createPinnedDispatcher (consoleUrl, resolvedIps) { +// Returns a dns.lookup-compatible function that resolves the console hostname +// exclusively to the addresses validated by validateConsoleUrl(). Passing it as +// the `lookup` option of http/https.request pins the connection to those +// addresses and closes the DNS-rebinding gap between validation and connect. +// Built-in modules only — this script must stay standalone (no node_modules). +function createPinnedLookup (consoleUrl, resolvedIps) { const expectedHostname = new URL(consoleUrl).hostname.toLowerCase().replace(/^\[/, '').replace(/\]$/, '').replace(/\.$/, '') const allowedIps = [...new Set(resolvedIps)] @@ -336,39 +341,34 @@ function createPinnedDispatcher (consoleUrl, resolvedIps) { throw new Error(`No validated addresses available for consoleUrl: ${consoleUrl}`) } - return new Agent({ - connect: { - autoSelectFamily: true, - lookup: (hostname, options, callback) => { - const requestedHostname = hostname.toLowerCase().replace(/\.$/, '') - if (requestedHostname !== expectedHostname) { - const error = new Error(`Refusing to resolve unvalidated hostname: ${hostname}`) - error.code = 'ENOTFOUND' - callback(error) - return - } - - const family = typeof options === 'number' ? options : options?.family - const matches = allowedIps - .map(address => ({ address, family: net.isIP(address) })) - .filter(record => record.family !== 0 && (!family || record.family === family)) - - if (matches.length === 0) { - const error = new Error(`No validated address matches the requested family for: ${hostname}`) - error.code = 'ENOTFOUND' - callback(error) - return - } - - if (typeof options === 'object' && options?.all) { - callback(null, matches) - return - } - - callback(null, matches[0].address, matches[0].family) - } + return (hostname, options, callback) => { + const requestedHostname = hostname.toLowerCase().replace(/\.$/, '') + if (requestedHostname !== expectedHostname) { + const error = new Error(`Refusing to resolve unvalidated hostname: ${hostname}`) + error.code = 'ENOTFOUND' + callback(error) + return } - }) + + const family = typeof options === 'number' ? options : options?.family + const matches = allowedIps + .map(address => ({ address, family: net.isIP(address) })) + .filter(record => record.family !== 0 && (!family || record.family === family)) + + if (matches.length === 0) { + const error = new Error(`No validated address matches the requested family for: ${hostname}`) + error.code = 'ENOTFOUND' + callback(error) + return + } + + if (typeof options === 'object' && options?.all) { + callback(null, matches) + return + } + + callback(null, matches[0].address, matches[0].family) + } } async function readCredentials () { @@ -402,48 +402,65 @@ async function readCredentials () { } async function downloadAsset (consoleUrl, token, assetId, destPath, validatedIps) { - const url = `${consoleUrl}/api/v3/asset/${encodeURIComponent(assetId)}` + const url = new URL(`${consoleUrl}/api/v3/asset/${encodeURIComponent(assetId)}`) console.log(`Fetching asset from: ${url}`) const resolvedIps = validatedIps === undefined ? await validateConsoleUrl(consoleUrl) : validatedIps - const dispatcher = resolvedIps === null + const lookup = resolvedIps === null ? undefined - : createPinnedDispatcher(consoleUrl, resolvedIps) + : createPinnedLookup(consoleUrl, resolvedIps) + + // Keep incomplete bytes invisible to resolveExistingAsset(). The temporary + // file lives beside the destination so renameSync publishes it atomically. + const tempPath = `${destPath}.${process.pid}.${randomUUID()}.tmp` + + // One absolute deadline spanning connect, response headers, and body — the + // same total budget AbortSignal.timeout(600_000) enforced before. + // req.setTimeout() would only measure socket inactivity, so a stalled + // connection could otherwise exceed the budget indefinitely. + let abortInFlight = () => {} + const deadline = setTimeout(() => { + abortInFlight(new Error(`Download of asset ${assetId} exceeded 10 minutes`)) + }, 600_000) try { - const res = await fetch(url, { - headers: { - 'x-nsolid-service-token': token, - Accept: 'application/json' - }, - redirect: 'error', - dispatcher, - // 10-minute total budget: large snapshots (>256MB) over slow links - // must not abort at 120s mid-body. - signal: AbortSignal.timeout(600_000) + const res = await new Promise((resolve, reject) => { + // http/https.request never follows redirects, so the service token can + // never be forwarded to a different origin. + const transport = url.protocol === 'http:' ? http : https + const req = transport.request(url, { + headers: { + 'x-nsolid-service-token': token, + Accept: 'application/json' + }, + lookup + }, resolve) + abortInFlight = (error) => req.destroy(error) + req.on('error', reject) + req.end() }) - if (!res.ok) { - await res.body?.cancel() - throw new Error(`Console returned ${res.status} ${res.statusText} for asset ${assetId}`) + if (res.statusCode < 200 || res.statusCode >= 300) { + res.resume() // drain the socket before throwing + const isRedirect = res.statusCode >= 300 && res.statusCode < 400 + throw new Error( + `Console returned ${res.statusCode} ${res.statusMessage} for asset ${assetId}` + + (isRedirect ? ' (redirects are not followed)' : '') + ) } - // Keep incomplete bytes invisible to resolveExistingAsset(). The temporary - // file lives beside the destination so renameSync publishes it atomically. - const tempPath = `${destPath}.${process.pid}.${randomUUID()}.tmp` - try { - await pipeline(Readable.fromWeb(res.body), fs.createWriteStream(tempPath, { flags: 'wx' })) - fs.renameSync(tempPath, destPath) - } catch (error) { - fs.rmSync(tempPath, { force: true }) - throw error - } + abortInFlight = (error) => res.destroy(error) + await pipeline(res, fs.createWriteStream(tempPath, { flags: 'wx' })) + fs.renameSync(tempPath, destPath) return fs.statSync(destPath).size + } catch (error) { + fs.rmSync(tempPath, { force: true }) + throw error } finally { - await dispatcher?.close() + clearTimeout(deadline) } } diff --git a/skills/ns-analyze-asset/fetch-asset.cjs b/skills/ns-analyze-asset/fetch-asset.cjs index 3f72628..b3090f2 100644 --- a/skills/ns-analyze-asset/fetch-asset.cjs +++ b/skills/ns-analyze-asset/fetch-asset.cjs @@ -29,8 +29,8 @@ const dns = require('dns').promises const net = require('net') const { randomUUID } = require('crypto') const { pipeline } = require('stream/promises') -const { Readable } = require('stream') -const { Agent } = require('undici') +const http = require('http') +const https = require('https') const EXTENSIONS = { cpuprofile: '.cpuprofile', @@ -328,7 +328,12 @@ async function validateConsoleUrl (consoleUrl) { return ips } -function createPinnedDispatcher (consoleUrl, resolvedIps) { +// Returns a dns.lookup-compatible function that resolves the console hostname +// exclusively to the addresses validated by validateConsoleUrl(). Passing it as +// the `lookup` option of http/https.request pins the connection to those +// addresses and closes the DNS-rebinding gap between validation and connect. +// Built-in modules only — this script must stay standalone (no node_modules). +function createPinnedLookup (consoleUrl, resolvedIps) { const expectedHostname = new URL(consoleUrl).hostname.toLowerCase().replace(/^\[/, '').replace(/\]$/, '').replace(/\.$/, '') const allowedIps = [...new Set(resolvedIps)] @@ -336,39 +341,34 @@ function createPinnedDispatcher (consoleUrl, resolvedIps) { throw new Error(`No validated addresses available for consoleUrl: ${consoleUrl}`) } - return new Agent({ - connect: { - autoSelectFamily: true, - lookup: (hostname, options, callback) => { - const requestedHostname = hostname.toLowerCase().replace(/\.$/, '') - if (requestedHostname !== expectedHostname) { - const error = new Error(`Refusing to resolve unvalidated hostname: ${hostname}`) - error.code = 'ENOTFOUND' - callback(error) - return - } - - const family = typeof options === 'number' ? options : options?.family - const matches = allowedIps - .map(address => ({ address, family: net.isIP(address) })) - .filter(record => record.family !== 0 && (!family || record.family === family)) - - if (matches.length === 0) { - const error = new Error(`No validated address matches the requested family for: ${hostname}`) - error.code = 'ENOTFOUND' - callback(error) - return - } - - if (typeof options === 'object' && options?.all) { - callback(null, matches) - return - } - - callback(null, matches[0].address, matches[0].family) - } + return (hostname, options, callback) => { + const requestedHostname = hostname.toLowerCase().replace(/\.$/, '') + if (requestedHostname !== expectedHostname) { + const error = new Error(`Refusing to resolve unvalidated hostname: ${hostname}`) + error.code = 'ENOTFOUND' + callback(error) + return } - }) + + const family = typeof options === 'number' ? options : options?.family + const matches = allowedIps + .map(address => ({ address, family: net.isIP(address) })) + .filter(record => record.family !== 0 && (!family || record.family === family)) + + if (matches.length === 0) { + const error = new Error(`No validated address matches the requested family for: ${hostname}`) + error.code = 'ENOTFOUND' + callback(error) + return + } + + if (typeof options === 'object' && options?.all) { + callback(null, matches) + return + } + + callback(null, matches[0].address, matches[0].family) + } } async function readCredentials () { @@ -402,48 +402,65 @@ async function readCredentials () { } async function downloadAsset (consoleUrl, token, assetId, destPath, validatedIps) { - const url = `${consoleUrl}/api/v3/asset/${encodeURIComponent(assetId)}` + const url = new URL(`${consoleUrl}/api/v3/asset/${encodeURIComponent(assetId)}`) console.log(`Fetching asset from: ${url}`) const resolvedIps = validatedIps === undefined ? await validateConsoleUrl(consoleUrl) : validatedIps - const dispatcher = resolvedIps === null + const lookup = resolvedIps === null ? undefined - : createPinnedDispatcher(consoleUrl, resolvedIps) + : createPinnedLookup(consoleUrl, resolvedIps) + + // Keep incomplete bytes invisible to resolveExistingAsset(). The temporary + // file lives beside the destination so renameSync publishes it atomically. + const tempPath = `${destPath}.${process.pid}.${randomUUID()}.tmp` + + // One absolute deadline spanning connect, response headers, and body — the + // same total budget AbortSignal.timeout(600_000) enforced before. + // req.setTimeout() would only measure socket inactivity, so a stalled + // connection could otherwise exceed the budget indefinitely. + let abortInFlight = () => {} + const deadline = setTimeout(() => { + abortInFlight(new Error(`Download of asset ${assetId} exceeded 10 minutes`)) + }, 600_000) try { - const res = await fetch(url, { - headers: { - 'x-nsolid-service-token': token, - Accept: 'application/json' - }, - redirect: 'error', - dispatcher, - // 10-minute total budget: large snapshots (>256MB) over slow links - // must not abort at 120s mid-body. - signal: AbortSignal.timeout(600_000) + const res = await new Promise((resolve, reject) => { + // http/https.request never follows redirects, so the service token can + // never be forwarded to a different origin. + const transport = url.protocol === 'http:' ? http : https + const req = transport.request(url, { + headers: { + 'x-nsolid-service-token': token, + Accept: 'application/json' + }, + lookup + }, resolve) + abortInFlight = (error) => req.destroy(error) + req.on('error', reject) + req.end() }) - if (!res.ok) { - await res.body?.cancel() - throw new Error(`Console returned ${res.status} ${res.statusText} for asset ${assetId}`) + if (res.statusCode < 200 || res.statusCode >= 300) { + res.resume() // drain the socket before throwing + const isRedirect = res.statusCode >= 300 && res.statusCode < 400 + throw new Error( + `Console returned ${res.statusCode} ${res.statusMessage} for asset ${assetId}` + + (isRedirect ? ' (redirects are not followed)' : '') + ) } - // Keep incomplete bytes invisible to resolveExistingAsset(). The temporary - // file lives beside the destination so renameSync publishes it atomically. - const tempPath = `${destPath}.${process.pid}.${randomUUID()}.tmp` - try { - await pipeline(Readable.fromWeb(res.body), fs.createWriteStream(tempPath, { flags: 'wx' })) - fs.renameSync(tempPath, destPath) - } catch (error) { - fs.rmSync(tempPath, { force: true }) - throw error - } + abortInFlight = (error) => res.destroy(error) + await pipeline(res, fs.createWriteStream(tempPath, { flags: 'wx' })) + fs.renameSync(tempPath, destPath) return fs.statSync(destPath).size + } catch (error) { + fs.rmSync(tempPath, { force: true }) + throw error } finally { - await dispatcher?.close() + clearTimeout(deadline) } } diff --git a/skills/ns-cpu-spike-analysis/fetch-asset.cjs b/skills/ns-cpu-spike-analysis/fetch-asset.cjs index 3f72628..b3090f2 100644 --- a/skills/ns-cpu-spike-analysis/fetch-asset.cjs +++ b/skills/ns-cpu-spike-analysis/fetch-asset.cjs @@ -29,8 +29,8 @@ const dns = require('dns').promises const net = require('net') const { randomUUID } = require('crypto') const { pipeline } = require('stream/promises') -const { Readable } = require('stream') -const { Agent } = require('undici') +const http = require('http') +const https = require('https') const EXTENSIONS = { cpuprofile: '.cpuprofile', @@ -328,7 +328,12 @@ async function validateConsoleUrl (consoleUrl) { return ips } -function createPinnedDispatcher (consoleUrl, resolvedIps) { +// Returns a dns.lookup-compatible function that resolves the console hostname +// exclusively to the addresses validated by validateConsoleUrl(). Passing it as +// the `lookup` option of http/https.request pins the connection to those +// addresses and closes the DNS-rebinding gap between validation and connect. +// Built-in modules only — this script must stay standalone (no node_modules). +function createPinnedLookup (consoleUrl, resolvedIps) { const expectedHostname = new URL(consoleUrl).hostname.toLowerCase().replace(/^\[/, '').replace(/\]$/, '').replace(/\.$/, '') const allowedIps = [...new Set(resolvedIps)] @@ -336,39 +341,34 @@ function createPinnedDispatcher (consoleUrl, resolvedIps) { throw new Error(`No validated addresses available for consoleUrl: ${consoleUrl}`) } - return new Agent({ - connect: { - autoSelectFamily: true, - lookup: (hostname, options, callback) => { - const requestedHostname = hostname.toLowerCase().replace(/\.$/, '') - if (requestedHostname !== expectedHostname) { - const error = new Error(`Refusing to resolve unvalidated hostname: ${hostname}`) - error.code = 'ENOTFOUND' - callback(error) - return - } - - const family = typeof options === 'number' ? options : options?.family - const matches = allowedIps - .map(address => ({ address, family: net.isIP(address) })) - .filter(record => record.family !== 0 && (!family || record.family === family)) - - if (matches.length === 0) { - const error = new Error(`No validated address matches the requested family for: ${hostname}`) - error.code = 'ENOTFOUND' - callback(error) - return - } - - if (typeof options === 'object' && options?.all) { - callback(null, matches) - return - } - - callback(null, matches[0].address, matches[0].family) - } + return (hostname, options, callback) => { + const requestedHostname = hostname.toLowerCase().replace(/\.$/, '') + if (requestedHostname !== expectedHostname) { + const error = new Error(`Refusing to resolve unvalidated hostname: ${hostname}`) + error.code = 'ENOTFOUND' + callback(error) + return } - }) + + const family = typeof options === 'number' ? options : options?.family + const matches = allowedIps + .map(address => ({ address, family: net.isIP(address) })) + .filter(record => record.family !== 0 && (!family || record.family === family)) + + if (matches.length === 0) { + const error = new Error(`No validated address matches the requested family for: ${hostname}`) + error.code = 'ENOTFOUND' + callback(error) + return + } + + if (typeof options === 'object' && options?.all) { + callback(null, matches) + return + } + + callback(null, matches[0].address, matches[0].family) + } } async function readCredentials () { @@ -402,48 +402,65 @@ async function readCredentials () { } async function downloadAsset (consoleUrl, token, assetId, destPath, validatedIps) { - const url = `${consoleUrl}/api/v3/asset/${encodeURIComponent(assetId)}` + const url = new URL(`${consoleUrl}/api/v3/asset/${encodeURIComponent(assetId)}`) console.log(`Fetching asset from: ${url}`) const resolvedIps = validatedIps === undefined ? await validateConsoleUrl(consoleUrl) : validatedIps - const dispatcher = resolvedIps === null + const lookup = resolvedIps === null ? undefined - : createPinnedDispatcher(consoleUrl, resolvedIps) + : createPinnedLookup(consoleUrl, resolvedIps) + + // Keep incomplete bytes invisible to resolveExistingAsset(). The temporary + // file lives beside the destination so renameSync publishes it atomically. + const tempPath = `${destPath}.${process.pid}.${randomUUID()}.tmp` + + // One absolute deadline spanning connect, response headers, and body — the + // same total budget AbortSignal.timeout(600_000) enforced before. + // req.setTimeout() would only measure socket inactivity, so a stalled + // connection could otherwise exceed the budget indefinitely. + let abortInFlight = () => {} + const deadline = setTimeout(() => { + abortInFlight(new Error(`Download of asset ${assetId} exceeded 10 minutes`)) + }, 600_000) try { - const res = await fetch(url, { - headers: { - 'x-nsolid-service-token': token, - Accept: 'application/json' - }, - redirect: 'error', - dispatcher, - // 10-minute total budget: large snapshots (>256MB) over slow links - // must not abort at 120s mid-body. - signal: AbortSignal.timeout(600_000) + const res = await new Promise((resolve, reject) => { + // http/https.request never follows redirects, so the service token can + // never be forwarded to a different origin. + const transport = url.protocol === 'http:' ? http : https + const req = transport.request(url, { + headers: { + 'x-nsolid-service-token': token, + Accept: 'application/json' + }, + lookup + }, resolve) + abortInFlight = (error) => req.destroy(error) + req.on('error', reject) + req.end() }) - if (!res.ok) { - await res.body?.cancel() - throw new Error(`Console returned ${res.status} ${res.statusText} for asset ${assetId}`) + if (res.statusCode < 200 || res.statusCode >= 300) { + res.resume() // drain the socket before throwing + const isRedirect = res.statusCode >= 300 && res.statusCode < 400 + throw new Error( + `Console returned ${res.statusCode} ${res.statusMessage} for asset ${assetId}` + + (isRedirect ? ' (redirects are not followed)' : '') + ) } - // Keep incomplete bytes invisible to resolveExistingAsset(). The temporary - // file lives beside the destination so renameSync publishes it atomically. - const tempPath = `${destPath}.${process.pid}.${randomUUID()}.tmp` - try { - await pipeline(Readable.fromWeb(res.body), fs.createWriteStream(tempPath, { flags: 'wx' })) - fs.renameSync(tempPath, destPath) - } catch (error) { - fs.rmSync(tempPath, { force: true }) - throw error - } + abortInFlight = (error) => res.destroy(error) + await pipeline(res, fs.createWriteStream(tempPath, { flags: 'wx' })) + fs.renameSync(tempPath, destPath) return fs.statSync(destPath).size + } catch (error) { + fs.rmSync(tempPath, { force: true }) + throw error } finally { - await dispatcher?.close() + clearTimeout(deadline) } } diff --git a/skills/ns-download-asset/fetch-asset.cjs b/skills/ns-download-asset/fetch-asset.cjs index 3f72628..b3090f2 100644 --- a/skills/ns-download-asset/fetch-asset.cjs +++ b/skills/ns-download-asset/fetch-asset.cjs @@ -29,8 +29,8 @@ const dns = require('dns').promises const net = require('net') const { randomUUID } = require('crypto') const { pipeline } = require('stream/promises') -const { Readable } = require('stream') -const { Agent } = require('undici') +const http = require('http') +const https = require('https') const EXTENSIONS = { cpuprofile: '.cpuprofile', @@ -328,7 +328,12 @@ async function validateConsoleUrl (consoleUrl) { return ips } -function createPinnedDispatcher (consoleUrl, resolvedIps) { +// Returns a dns.lookup-compatible function that resolves the console hostname +// exclusively to the addresses validated by validateConsoleUrl(). Passing it as +// the `lookup` option of http/https.request pins the connection to those +// addresses and closes the DNS-rebinding gap between validation and connect. +// Built-in modules only — this script must stay standalone (no node_modules). +function createPinnedLookup (consoleUrl, resolvedIps) { const expectedHostname = new URL(consoleUrl).hostname.toLowerCase().replace(/^\[/, '').replace(/\]$/, '').replace(/\.$/, '') const allowedIps = [...new Set(resolvedIps)] @@ -336,39 +341,34 @@ function createPinnedDispatcher (consoleUrl, resolvedIps) { throw new Error(`No validated addresses available for consoleUrl: ${consoleUrl}`) } - return new Agent({ - connect: { - autoSelectFamily: true, - lookup: (hostname, options, callback) => { - const requestedHostname = hostname.toLowerCase().replace(/\.$/, '') - if (requestedHostname !== expectedHostname) { - const error = new Error(`Refusing to resolve unvalidated hostname: ${hostname}`) - error.code = 'ENOTFOUND' - callback(error) - return - } - - const family = typeof options === 'number' ? options : options?.family - const matches = allowedIps - .map(address => ({ address, family: net.isIP(address) })) - .filter(record => record.family !== 0 && (!family || record.family === family)) - - if (matches.length === 0) { - const error = new Error(`No validated address matches the requested family for: ${hostname}`) - error.code = 'ENOTFOUND' - callback(error) - return - } - - if (typeof options === 'object' && options?.all) { - callback(null, matches) - return - } - - callback(null, matches[0].address, matches[0].family) - } + return (hostname, options, callback) => { + const requestedHostname = hostname.toLowerCase().replace(/\.$/, '') + if (requestedHostname !== expectedHostname) { + const error = new Error(`Refusing to resolve unvalidated hostname: ${hostname}`) + error.code = 'ENOTFOUND' + callback(error) + return } - }) + + const family = typeof options === 'number' ? options : options?.family + const matches = allowedIps + .map(address => ({ address, family: net.isIP(address) })) + .filter(record => record.family !== 0 && (!family || record.family === family)) + + if (matches.length === 0) { + const error = new Error(`No validated address matches the requested family for: ${hostname}`) + error.code = 'ENOTFOUND' + callback(error) + return + } + + if (typeof options === 'object' && options?.all) { + callback(null, matches) + return + } + + callback(null, matches[0].address, matches[0].family) + } } async function readCredentials () { @@ -402,48 +402,65 @@ async function readCredentials () { } async function downloadAsset (consoleUrl, token, assetId, destPath, validatedIps) { - const url = `${consoleUrl}/api/v3/asset/${encodeURIComponent(assetId)}` + const url = new URL(`${consoleUrl}/api/v3/asset/${encodeURIComponent(assetId)}`) console.log(`Fetching asset from: ${url}`) const resolvedIps = validatedIps === undefined ? await validateConsoleUrl(consoleUrl) : validatedIps - const dispatcher = resolvedIps === null + const lookup = resolvedIps === null ? undefined - : createPinnedDispatcher(consoleUrl, resolvedIps) + : createPinnedLookup(consoleUrl, resolvedIps) + + // Keep incomplete bytes invisible to resolveExistingAsset(). The temporary + // file lives beside the destination so renameSync publishes it atomically. + const tempPath = `${destPath}.${process.pid}.${randomUUID()}.tmp` + + // One absolute deadline spanning connect, response headers, and body — the + // same total budget AbortSignal.timeout(600_000) enforced before. + // req.setTimeout() would only measure socket inactivity, so a stalled + // connection could otherwise exceed the budget indefinitely. + let abortInFlight = () => {} + const deadline = setTimeout(() => { + abortInFlight(new Error(`Download of asset ${assetId} exceeded 10 minutes`)) + }, 600_000) try { - const res = await fetch(url, { - headers: { - 'x-nsolid-service-token': token, - Accept: 'application/json' - }, - redirect: 'error', - dispatcher, - // 10-minute total budget: large snapshots (>256MB) over slow links - // must not abort at 120s mid-body. - signal: AbortSignal.timeout(600_000) + const res = await new Promise((resolve, reject) => { + // http/https.request never follows redirects, so the service token can + // never be forwarded to a different origin. + const transport = url.protocol === 'http:' ? http : https + const req = transport.request(url, { + headers: { + 'x-nsolid-service-token': token, + Accept: 'application/json' + }, + lookup + }, resolve) + abortInFlight = (error) => req.destroy(error) + req.on('error', reject) + req.end() }) - if (!res.ok) { - await res.body?.cancel() - throw new Error(`Console returned ${res.status} ${res.statusText} for asset ${assetId}`) + if (res.statusCode < 200 || res.statusCode >= 300) { + res.resume() // drain the socket before throwing + const isRedirect = res.statusCode >= 300 && res.statusCode < 400 + throw new Error( + `Console returned ${res.statusCode} ${res.statusMessage} for asset ${assetId}` + + (isRedirect ? ' (redirects are not followed)' : '') + ) } - // Keep incomplete bytes invisible to resolveExistingAsset(). The temporary - // file lives beside the destination so renameSync publishes it atomically. - const tempPath = `${destPath}.${process.pid}.${randomUUID()}.tmp` - try { - await pipeline(Readable.fromWeb(res.body), fs.createWriteStream(tempPath, { flags: 'wx' })) - fs.renameSync(tempPath, destPath) - } catch (error) { - fs.rmSync(tempPath, { force: true }) - throw error - } + abortInFlight = (error) => res.destroy(error) + await pipeline(res, fs.createWriteStream(tempPath, { flags: 'wx' })) + fs.renameSync(tempPath, destPath) return fs.statSync(destPath).size + } catch (error) { + fs.rmSync(tempPath, { force: true }) + throw error } finally { - await dispatcher?.close() + clearTimeout(deadline) } } diff --git a/skills/ns-generate-asset/fetch-asset.cjs b/skills/ns-generate-asset/fetch-asset.cjs index 3f72628..b3090f2 100644 --- a/skills/ns-generate-asset/fetch-asset.cjs +++ b/skills/ns-generate-asset/fetch-asset.cjs @@ -29,8 +29,8 @@ const dns = require('dns').promises const net = require('net') const { randomUUID } = require('crypto') const { pipeline } = require('stream/promises') -const { Readable } = require('stream') -const { Agent } = require('undici') +const http = require('http') +const https = require('https') const EXTENSIONS = { cpuprofile: '.cpuprofile', @@ -328,7 +328,12 @@ async function validateConsoleUrl (consoleUrl) { return ips } -function createPinnedDispatcher (consoleUrl, resolvedIps) { +// Returns a dns.lookup-compatible function that resolves the console hostname +// exclusively to the addresses validated by validateConsoleUrl(). Passing it as +// the `lookup` option of http/https.request pins the connection to those +// addresses and closes the DNS-rebinding gap between validation and connect. +// Built-in modules only — this script must stay standalone (no node_modules). +function createPinnedLookup (consoleUrl, resolvedIps) { const expectedHostname = new URL(consoleUrl).hostname.toLowerCase().replace(/^\[/, '').replace(/\]$/, '').replace(/\.$/, '') const allowedIps = [...new Set(resolvedIps)] @@ -336,39 +341,34 @@ function createPinnedDispatcher (consoleUrl, resolvedIps) { throw new Error(`No validated addresses available for consoleUrl: ${consoleUrl}`) } - return new Agent({ - connect: { - autoSelectFamily: true, - lookup: (hostname, options, callback) => { - const requestedHostname = hostname.toLowerCase().replace(/\.$/, '') - if (requestedHostname !== expectedHostname) { - const error = new Error(`Refusing to resolve unvalidated hostname: ${hostname}`) - error.code = 'ENOTFOUND' - callback(error) - return - } - - const family = typeof options === 'number' ? options : options?.family - const matches = allowedIps - .map(address => ({ address, family: net.isIP(address) })) - .filter(record => record.family !== 0 && (!family || record.family === family)) - - if (matches.length === 0) { - const error = new Error(`No validated address matches the requested family for: ${hostname}`) - error.code = 'ENOTFOUND' - callback(error) - return - } - - if (typeof options === 'object' && options?.all) { - callback(null, matches) - return - } - - callback(null, matches[0].address, matches[0].family) - } + return (hostname, options, callback) => { + const requestedHostname = hostname.toLowerCase().replace(/\.$/, '') + if (requestedHostname !== expectedHostname) { + const error = new Error(`Refusing to resolve unvalidated hostname: ${hostname}`) + error.code = 'ENOTFOUND' + callback(error) + return } - }) + + const family = typeof options === 'number' ? options : options?.family + const matches = allowedIps + .map(address => ({ address, family: net.isIP(address) })) + .filter(record => record.family !== 0 && (!family || record.family === family)) + + if (matches.length === 0) { + const error = new Error(`No validated address matches the requested family for: ${hostname}`) + error.code = 'ENOTFOUND' + callback(error) + return + } + + if (typeof options === 'object' && options?.all) { + callback(null, matches) + return + } + + callback(null, matches[0].address, matches[0].family) + } } async function readCredentials () { @@ -402,48 +402,65 @@ async function readCredentials () { } async function downloadAsset (consoleUrl, token, assetId, destPath, validatedIps) { - const url = `${consoleUrl}/api/v3/asset/${encodeURIComponent(assetId)}` + const url = new URL(`${consoleUrl}/api/v3/asset/${encodeURIComponent(assetId)}`) console.log(`Fetching asset from: ${url}`) const resolvedIps = validatedIps === undefined ? await validateConsoleUrl(consoleUrl) : validatedIps - const dispatcher = resolvedIps === null + const lookup = resolvedIps === null ? undefined - : createPinnedDispatcher(consoleUrl, resolvedIps) + : createPinnedLookup(consoleUrl, resolvedIps) + + // Keep incomplete bytes invisible to resolveExistingAsset(). The temporary + // file lives beside the destination so renameSync publishes it atomically. + const tempPath = `${destPath}.${process.pid}.${randomUUID()}.tmp` + + // One absolute deadline spanning connect, response headers, and body — the + // same total budget AbortSignal.timeout(600_000) enforced before. + // req.setTimeout() would only measure socket inactivity, so a stalled + // connection could otherwise exceed the budget indefinitely. + let abortInFlight = () => {} + const deadline = setTimeout(() => { + abortInFlight(new Error(`Download of asset ${assetId} exceeded 10 minutes`)) + }, 600_000) try { - const res = await fetch(url, { - headers: { - 'x-nsolid-service-token': token, - Accept: 'application/json' - }, - redirect: 'error', - dispatcher, - // 10-minute total budget: large snapshots (>256MB) over slow links - // must not abort at 120s mid-body. - signal: AbortSignal.timeout(600_000) + const res = await new Promise((resolve, reject) => { + // http/https.request never follows redirects, so the service token can + // never be forwarded to a different origin. + const transport = url.protocol === 'http:' ? http : https + const req = transport.request(url, { + headers: { + 'x-nsolid-service-token': token, + Accept: 'application/json' + }, + lookup + }, resolve) + abortInFlight = (error) => req.destroy(error) + req.on('error', reject) + req.end() }) - if (!res.ok) { - await res.body?.cancel() - throw new Error(`Console returned ${res.status} ${res.statusText} for asset ${assetId}`) + if (res.statusCode < 200 || res.statusCode >= 300) { + res.resume() // drain the socket before throwing + const isRedirect = res.statusCode >= 300 && res.statusCode < 400 + throw new Error( + `Console returned ${res.statusCode} ${res.statusMessage} for asset ${assetId}` + + (isRedirect ? ' (redirects are not followed)' : '') + ) } - // Keep incomplete bytes invisible to resolveExistingAsset(). The temporary - // file lives beside the destination so renameSync publishes it atomically. - const tempPath = `${destPath}.${process.pid}.${randomUUID()}.tmp` - try { - await pipeline(Readable.fromWeb(res.body), fs.createWriteStream(tempPath, { flags: 'wx' })) - fs.renameSync(tempPath, destPath) - } catch (error) { - fs.rmSync(tempPath, { force: true }) - throw error - } + abortInFlight = (error) => res.destroy(error) + await pipeline(res, fs.createWriteStream(tempPath, { flags: 'wx' })) + fs.renameSync(tempPath, destPath) return fs.statSync(destPath).size + } catch (error) { + fs.rmSync(tempPath, { force: true }) + throw error } finally { - await dispatcher?.close() + clearTimeout(deadline) } } diff --git a/skills/ns-memory-spike-analysis/fetch-asset.cjs b/skills/ns-memory-spike-analysis/fetch-asset.cjs index 3f72628..b3090f2 100644 --- a/skills/ns-memory-spike-analysis/fetch-asset.cjs +++ b/skills/ns-memory-spike-analysis/fetch-asset.cjs @@ -29,8 +29,8 @@ const dns = require('dns').promises const net = require('net') const { randomUUID } = require('crypto') const { pipeline } = require('stream/promises') -const { Readable } = require('stream') -const { Agent } = require('undici') +const http = require('http') +const https = require('https') const EXTENSIONS = { cpuprofile: '.cpuprofile', @@ -328,7 +328,12 @@ async function validateConsoleUrl (consoleUrl) { return ips } -function createPinnedDispatcher (consoleUrl, resolvedIps) { +// Returns a dns.lookup-compatible function that resolves the console hostname +// exclusively to the addresses validated by validateConsoleUrl(). Passing it as +// the `lookup` option of http/https.request pins the connection to those +// addresses and closes the DNS-rebinding gap between validation and connect. +// Built-in modules only — this script must stay standalone (no node_modules). +function createPinnedLookup (consoleUrl, resolvedIps) { const expectedHostname = new URL(consoleUrl).hostname.toLowerCase().replace(/^\[/, '').replace(/\]$/, '').replace(/\.$/, '') const allowedIps = [...new Set(resolvedIps)] @@ -336,39 +341,34 @@ function createPinnedDispatcher (consoleUrl, resolvedIps) { throw new Error(`No validated addresses available for consoleUrl: ${consoleUrl}`) } - return new Agent({ - connect: { - autoSelectFamily: true, - lookup: (hostname, options, callback) => { - const requestedHostname = hostname.toLowerCase().replace(/\.$/, '') - if (requestedHostname !== expectedHostname) { - const error = new Error(`Refusing to resolve unvalidated hostname: ${hostname}`) - error.code = 'ENOTFOUND' - callback(error) - return - } - - const family = typeof options === 'number' ? options : options?.family - const matches = allowedIps - .map(address => ({ address, family: net.isIP(address) })) - .filter(record => record.family !== 0 && (!family || record.family === family)) - - if (matches.length === 0) { - const error = new Error(`No validated address matches the requested family for: ${hostname}`) - error.code = 'ENOTFOUND' - callback(error) - return - } - - if (typeof options === 'object' && options?.all) { - callback(null, matches) - return - } - - callback(null, matches[0].address, matches[0].family) - } + return (hostname, options, callback) => { + const requestedHostname = hostname.toLowerCase().replace(/\.$/, '') + if (requestedHostname !== expectedHostname) { + const error = new Error(`Refusing to resolve unvalidated hostname: ${hostname}`) + error.code = 'ENOTFOUND' + callback(error) + return } - }) + + const family = typeof options === 'number' ? options : options?.family + const matches = allowedIps + .map(address => ({ address, family: net.isIP(address) })) + .filter(record => record.family !== 0 && (!family || record.family === family)) + + if (matches.length === 0) { + const error = new Error(`No validated address matches the requested family for: ${hostname}`) + error.code = 'ENOTFOUND' + callback(error) + return + } + + if (typeof options === 'object' && options?.all) { + callback(null, matches) + return + } + + callback(null, matches[0].address, matches[0].family) + } } async function readCredentials () { @@ -402,48 +402,65 @@ async function readCredentials () { } async function downloadAsset (consoleUrl, token, assetId, destPath, validatedIps) { - const url = `${consoleUrl}/api/v3/asset/${encodeURIComponent(assetId)}` + const url = new URL(`${consoleUrl}/api/v3/asset/${encodeURIComponent(assetId)}`) console.log(`Fetching asset from: ${url}`) const resolvedIps = validatedIps === undefined ? await validateConsoleUrl(consoleUrl) : validatedIps - const dispatcher = resolvedIps === null + const lookup = resolvedIps === null ? undefined - : createPinnedDispatcher(consoleUrl, resolvedIps) + : createPinnedLookup(consoleUrl, resolvedIps) + + // Keep incomplete bytes invisible to resolveExistingAsset(). The temporary + // file lives beside the destination so renameSync publishes it atomically. + const tempPath = `${destPath}.${process.pid}.${randomUUID()}.tmp` + + // One absolute deadline spanning connect, response headers, and body — the + // same total budget AbortSignal.timeout(600_000) enforced before. + // req.setTimeout() would only measure socket inactivity, so a stalled + // connection could otherwise exceed the budget indefinitely. + let abortInFlight = () => {} + const deadline = setTimeout(() => { + abortInFlight(new Error(`Download of asset ${assetId} exceeded 10 minutes`)) + }, 600_000) try { - const res = await fetch(url, { - headers: { - 'x-nsolid-service-token': token, - Accept: 'application/json' - }, - redirect: 'error', - dispatcher, - // 10-minute total budget: large snapshots (>256MB) over slow links - // must not abort at 120s mid-body. - signal: AbortSignal.timeout(600_000) + const res = await new Promise((resolve, reject) => { + // http/https.request never follows redirects, so the service token can + // never be forwarded to a different origin. + const transport = url.protocol === 'http:' ? http : https + const req = transport.request(url, { + headers: { + 'x-nsolid-service-token': token, + Accept: 'application/json' + }, + lookup + }, resolve) + abortInFlight = (error) => req.destroy(error) + req.on('error', reject) + req.end() }) - if (!res.ok) { - await res.body?.cancel() - throw new Error(`Console returned ${res.status} ${res.statusText} for asset ${assetId}`) + if (res.statusCode < 200 || res.statusCode >= 300) { + res.resume() // drain the socket before throwing + const isRedirect = res.statusCode >= 300 && res.statusCode < 400 + throw new Error( + `Console returned ${res.statusCode} ${res.statusMessage} for asset ${assetId}` + + (isRedirect ? ' (redirects are not followed)' : '') + ) } - // Keep incomplete bytes invisible to resolveExistingAsset(). The temporary - // file lives beside the destination so renameSync publishes it atomically. - const tempPath = `${destPath}.${process.pid}.${randomUUID()}.tmp` - try { - await pipeline(Readable.fromWeb(res.body), fs.createWriteStream(tempPath, { flags: 'wx' })) - fs.renameSync(tempPath, destPath) - } catch (error) { - fs.rmSync(tempPath, { force: true }) - throw error - } + abortInFlight = (error) => res.destroy(error) + await pipeline(res, fs.createWriteStream(tempPath, { flags: 'wx' })) + fs.renameSync(tempPath, destPath) return fs.statSync(destPath).size + } catch (error) { + fs.rmSync(tempPath, { force: true }) + throw error } finally { - await dispatcher?.close() + clearTimeout(deadline) } } From f40ac4a3d183bfcc1843e80d9c30d4e7c0341f3a Mon Sep 17 00:00:00 2001 From: Cesar-M-Diaz Date: Mon, 24 Aug 2026 17:21:49 +0200 Subject: [PATCH 4/8] test(skills): fix TS types in fetch-asset download mocks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit makeFakeRequest's respond callback is typed () => void, but call sites returned callback?.(...) (void | undefined) and the signature still expected () => IncomingMessage, breaking tsc (exit 2) in CI on all platforms. Tests passed locally because tsx skips typechecking. Also drop the stale req.setTimeout mock — the script now enforces a single absolute deadline and never calls it. --- .../core/test/unit/skills/fetch-asset.test.ts | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/packages/core/test/unit/skills/fetch-asset.test.ts b/packages/core/test/unit/skills/fetch-asset.test.ts index 1b5a605..4a73ea1 100644 --- a/packages/core/test/unit/skills/fetch-asset.test.ts +++ b/packages/core/test/unit/skills/fetch-asset.test.ts @@ -255,9 +255,8 @@ describe('validateConsoleUrl', () => { }) }) -function makeFakeRequest (respond: () => IncomingMessage) { +function makeFakeRequest (respond: () => void) { const req = new EventEmitter() as ClientRequest - req.setTimeout = () => req req.end = (() => { respond() return req @@ -316,7 +315,7 @@ describe('downloadAsset', () => { headers: (options.headers ?? {}) as Record, lookup: options.lookup }) - return makeFakeRequest(() => callback?.(makeFakeResponse(payload))) + return makeFakeRequest(() => { callback?.(makeFakeResponse(payload)) }) }) as typeof https.request) const { downloadAsset } = await loadFetchAsset() @@ -346,7 +345,7 @@ describe('downloadAsset', () => { const urls: string[] = [] t.mock.method(https, 'request', ((input: string | URL, _options: RequestOptions, callback?: (res: IncomingMessage) => void) => { urls.push(String(input)) - return makeFakeRequest(() => callback?.(makeFakeResponse(Buffer.from([1, 2, 3])))) + return makeFakeRequest(() => { callback?.(makeFakeResponse(Buffer.from([1, 2, 3]))) }) }) as typeof https.request) const { downloadAsset } = await loadFetchAsset() @@ -363,7 +362,7 @@ describe('downloadAsset', () => { it('throws on non-ok responses without creating a file', async (t) => { t.mock.method(https, 'request', ((_input: string | URL, _options: RequestOptions, callback?: (res: IncomingMessage) => void) => { - return makeFakeRequest(() => callback?.(makeFakeResponse(Buffer.from('not found'), 404, 'Not Found'))) + return makeFakeRequest(() => { callback?.(makeFakeResponse(Buffer.from('not found'), 404, 'Not Found')) }) }) as typeof https.request) const { downloadAsset } = await loadFetchAsset() @@ -377,10 +376,12 @@ describe('downloadAsset', () => { it('removes temporary bytes when the response stream fails', async (t) => { t.mock.method(https, 'request', ((_input: string | URL, _options: RequestOptions, callback?: (res: IncomingMessage) => void) => { - return makeFakeRequest(() => callback?.(makeFakeResponse(function (this: Readable) { - this.push(Buffer.from([1, 2, 3])) - queueMicrotask(() => this.destroy(new Error('stream interrupted'))) - }))) + return makeFakeRequest(() => { + callback?.(makeFakeResponse(function (this: Readable) { + this.push(Buffer.from([1, 2, 3])) + queueMicrotask(() => this.destroy(new Error('stream interrupted'))) + })) + }) }) as typeof https.request) const { downloadAsset } = await loadFetchAsset() From fcf48aafb4d1ddcf7647f2b63d68cca4269111c6 Mon Sep 17 00:00:00 2001 From: Cesar-M-Diaz Date: Mon, 24 Aug 2026 17:41:53 +0200 Subject: [PATCH 5/8] fix(skills): decompress gzip asset responses in fetch-asset.cjs The console can serve assets gzip-compressed regardless of Accept-Encoding negotiation (its 'compressed' flag). fetch() used to decompress transparently; the https.request rewrite wrote compressed bytes verbatim, corrupting the on-disk asset and its index fileSize. Detect Content-Encoding: gzip and insert zlib.createGunzip() into the pipeline; identity responses still stream straight to disk. Unknown encodings now fail loudly instead of writing garbage. Sends Accept-Encoding: identity so the common case stays uncompressed. Adds a regression test (gzip body -> decompressed file, decompressed size) and syncs the 6 skill copies. --- .../core/test/unit/skills/fetch-asset.test.ts | 28 ++++++++++++++++++- skill-assets/fetch-asset.cjs | 23 +++++++++++++-- .../fetch-asset.cjs | 23 +++++++++++++-- skills/ns-analyze-asset/fetch-asset.cjs | 23 +++++++++++++-- skills/ns-cpu-spike-analysis/fetch-asset.cjs | 23 +++++++++++++-- skills/ns-download-asset/fetch-asset.cjs | 23 +++++++++++++-- skills/ns-generate-asset/fetch-asset.cjs | 23 +++++++++++++-- .../ns-memory-spike-analysis/fetch-asset.cjs | 23 +++++++++++++-- 8 files changed, 174 insertions(+), 15 deletions(-) diff --git a/packages/core/test/unit/skills/fetch-asset.test.ts b/packages/core/test/unit/skills/fetch-asset.test.ts index 4a73ea1..6d5bc7d 100644 --- a/packages/core/test/unit/skills/fetch-asset.test.ts +++ b/packages/core/test/unit/skills/fetch-asset.test.ts @@ -8,6 +8,7 @@ import { tmpdir } from 'node:os' import https from 'node:https' import { EventEmitter } from 'node:events' import { Readable } from 'node:stream' +import { gzipSync } from 'node:zlib' import type { TestContext } from 'node:test' import type { ClientRequest, IncomingMessage, RequestOptions } from 'node:http' @@ -268,7 +269,7 @@ function makeFakeRequest (respond: () => void) { return req } -function makeFakeResponse (body: Buffer | ((this: Readable) => void), statusCode = 200, statusMessage = 'OK') { +function makeFakeResponse (body: Buffer | ((this: Readable) => void), statusCode = 200, statusMessage = 'OK', headers: Record = {}) { let readCount = 0 const res = new Readable({ read () { @@ -288,6 +289,7 @@ function makeFakeResponse (body: Buffer | ((this: Readable) => void), statusCode }) as IncomingMessage res.statusCode = statusCode res.statusMessage = statusMessage + res.headers = headers return res } @@ -395,6 +397,30 @@ describe('downloadAsset', () => { assert.deepEqual(await readdir(tempDir), [], 'does not leave a temporary file behind') }) + it('decompresses gzip-encoded responses before writing to disk', async (t) => { + const payload = Buffer.from('heap snapshot payload '.repeat(64)) + t.mock.method(https, 'request', ((_input: string | URL, _options: RequestOptions, callback?: (res: IncomingMessage) => void) => { + return makeFakeRequest(() => { + callback?.(makeFakeResponse(gzipSync(payload), 200, 'OK', { 'content-encoding': 'gzip' })) + }) + }) as typeof https.request) + + const { downloadAsset } = await loadFetchAsset() + const destPath = join(tempDir, 'gzipped.heapsnapshot') + + const size = await downloadAsset( + 'https://console.example.test', + 'token', + 'gzip-id', + destPath, + ['93.184.216.34'] + ) + + assert.equal(size, payload.byteLength, 'reports the decompressed size') + assert.deepEqual(await readFile(destPath), payload, 'file holds decompressed bytes') + assert.deepEqual(await readdir(tempDir), ['gzipped.heapsnapshot']) + }) + it('aborts when the whole exchange exceeds the 10-minute deadline', async (t) => { t.mock.timers.enable({ apis: ['setTimeout'] }) // The request never responds: covers connect/headers stalling past the diff --git a/skill-assets/fetch-asset.cjs b/skill-assets/fetch-asset.cjs index b3090f2..b029004 100644 --- a/skill-assets/fetch-asset.cjs +++ b/skill-assets/fetch-asset.cjs @@ -31,6 +31,7 @@ const { randomUUID } = require('crypto') const { pipeline } = require('stream/promises') const http = require('http') const https = require('https') +const zlib = require('zlib') const EXTENSIONS = { cpuprofile: '.cpuprofile', @@ -433,7 +434,10 @@ async function downloadAsset (consoleUrl, token, assetId, destPath, validatedIps const req = transport.request(url, { headers: { 'x-nsolid-service-token': token, - Accept: 'application/json' + Accept: 'application/json', + // We handle gzip manually below; ask the server for identity so the + // common case streams straight to disk. + 'Accept-Encoding': 'identity' }, lookup }, resolve) @@ -451,8 +455,23 @@ async function downloadAsset (consoleUrl, token, assetId, destPath, validatedIps ) } + // The console can serve assets gzip-compressed regardless of + // Accept-Encoding negotiation (its `compressed` flag). Decompress before + // writing so the on-disk asset and the recorded fileSize are always the + // plain payload — fetch() used to do this transparently. + const encoding = String(res.headers['content-encoding'] ?? 'identity').toLowerCase() + if (encoding !== 'identity' && encoding !== 'gzip') { + res.resume() // drain the socket before throwing + throw new Error(`Unsupported Content-Encoding "${encoding}" for asset ${assetId}`) + } + abortInFlight = (error) => res.destroy(error) - await pipeline(res, fs.createWriteStream(tempPath, { flags: 'wx' })) + const writer = fs.createWriteStream(tempPath, { flags: 'wx' }) + if (encoding === 'gzip') { + await pipeline(res, zlib.createGunzip(), writer) + } else { + await pipeline(res, writer) + } fs.renameSync(tempPath, destPath) return fs.statSync(destPath).size diff --git a/skills/ns-advanced-memory-leak-hunter/fetch-asset.cjs b/skills/ns-advanced-memory-leak-hunter/fetch-asset.cjs index b3090f2..b029004 100644 --- a/skills/ns-advanced-memory-leak-hunter/fetch-asset.cjs +++ b/skills/ns-advanced-memory-leak-hunter/fetch-asset.cjs @@ -31,6 +31,7 @@ const { randomUUID } = require('crypto') const { pipeline } = require('stream/promises') const http = require('http') const https = require('https') +const zlib = require('zlib') const EXTENSIONS = { cpuprofile: '.cpuprofile', @@ -433,7 +434,10 @@ async function downloadAsset (consoleUrl, token, assetId, destPath, validatedIps const req = transport.request(url, { headers: { 'x-nsolid-service-token': token, - Accept: 'application/json' + Accept: 'application/json', + // We handle gzip manually below; ask the server for identity so the + // common case streams straight to disk. + 'Accept-Encoding': 'identity' }, lookup }, resolve) @@ -451,8 +455,23 @@ async function downloadAsset (consoleUrl, token, assetId, destPath, validatedIps ) } + // The console can serve assets gzip-compressed regardless of + // Accept-Encoding negotiation (its `compressed` flag). Decompress before + // writing so the on-disk asset and the recorded fileSize are always the + // plain payload — fetch() used to do this transparently. + const encoding = String(res.headers['content-encoding'] ?? 'identity').toLowerCase() + if (encoding !== 'identity' && encoding !== 'gzip') { + res.resume() // drain the socket before throwing + throw new Error(`Unsupported Content-Encoding "${encoding}" for asset ${assetId}`) + } + abortInFlight = (error) => res.destroy(error) - await pipeline(res, fs.createWriteStream(tempPath, { flags: 'wx' })) + const writer = fs.createWriteStream(tempPath, { flags: 'wx' }) + if (encoding === 'gzip') { + await pipeline(res, zlib.createGunzip(), writer) + } else { + await pipeline(res, writer) + } fs.renameSync(tempPath, destPath) return fs.statSync(destPath).size diff --git a/skills/ns-analyze-asset/fetch-asset.cjs b/skills/ns-analyze-asset/fetch-asset.cjs index b3090f2..b029004 100644 --- a/skills/ns-analyze-asset/fetch-asset.cjs +++ b/skills/ns-analyze-asset/fetch-asset.cjs @@ -31,6 +31,7 @@ const { randomUUID } = require('crypto') const { pipeline } = require('stream/promises') const http = require('http') const https = require('https') +const zlib = require('zlib') const EXTENSIONS = { cpuprofile: '.cpuprofile', @@ -433,7 +434,10 @@ async function downloadAsset (consoleUrl, token, assetId, destPath, validatedIps const req = transport.request(url, { headers: { 'x-nsolid-service-token': token, - Accept: 'application/json' + Accept: 'application/json', + // We handle gzip manually below; ask the server for identity so the + // common case streams straight to disk. + 'Accept-Encoding': 'identity' }, lookup }, resolve) @@ -451,8 +455,23 @@ async function downloadAsset (consoleUrl, token, assetId, destPath, validatedIps ) } + // The console can serve assets gzip-compressed regardless of + // Accept-Encoding negotiation (its `compressed` flag). Decompress before + // writing so the on-disk asset and the recorded fileSize are always the + // plain payload — fetch() used to do this transparently. + const encoding = String(res.headers['content-encoding'] ?? 'identity').toLowerCase() + if (encoding !== 'identity' && encoding !== 'gzip') { + res.resume() // drain the socket before throwing + throw new Error(`Unsupported Content-Encoding "${encoding}" for asset ${assetId}`) + } + abortInFlight = (error) => res.destroy(error) - await pipeline(res, fs.createWriteStream(tempPath, { flags: 'wx' })) + const writer = fs.createWriteStream(tempPath, { flags: 'wx' }) + if (encoding === 'gzip') { + await pipeline(res, zlib.createGunzip(), writer) + } else { + await pipeline(res, writer) + } fs.renameSync(tempPath, destPath) return fs.statSync(destPath).size diff --git a/skills/ns-cpu-spike-analysis/fetch-asset.cjs b/skills/ns-cpu-spike-analysis/fetch-asset.cjs index b3090f2..b029004 100644 --- a/skills/ns-cpu-spike-analysis/fetch-asset.cjs +++ b/skills/ns-cpu-spike-analysis/fetch-asset.cjs @@ -31,6 +31,7 @@ const { randomUUID } = require('crypto') const { pipeline } = require('stream/promises') const http = require('http') const https = require('https') +const zlib = require('zlib') const EXTENSIONS = { cpuprofile: '.cpuprofile', @@ -433,7 +434,10 @@ async function downloadAsset (consoleUrl, token, assetId, destPath, validatedIps const req = transport.request(url, { headers: { 'x-nsolid-service-token': token, - Accept: 'application/json' + Accept: 'application/json', + // We handle gzip manually below; ask the server for identity so the + // common case streams straight to disk. + 'Accept-Encoding': 'identity' }, lookup }, resolve) @@ -451,8 +455,23 @@ async function downloadAsset (consoleUrl, token, assetId, destPath, validatedIps ) } + // The console can serve assets gzip-compressed regardless of + // Accept-Encoding negotiation (its `compressed` flag). Decompress before + // writing so the on-disk asset and the recorded fileSize are always the + // plain payload — fetch() used to do this transparently. + const encoding = String(res.headers['content-encoding'] ?? 'identity').toLowerCase() + if (encoding !== 'identity' && encoding !== 'gzip') { + res.resume() // drain the socket before throwing + throw new Error(`Unsupported Content-Encoding "${encoding}" for asset ${assetId}`) + } + abortInFlight = (error) => res.destroy(error) - await pipeline(res, fs.createWriteStream(tempPath, { flags: 'wx' })) + const writer = fs.createWriteStream(tempPath, { flags: 'wx' }) + if (encoding === 'gzip') { + await pipeline(res, zlib.createGunzip(), writer) + } else { + await pipeline(res, writer) + } fs.renameSync(tempPath, destPath) return fs.statSync(destPath).size diff --git a/skills/ns-download-asset/fetch-asset.cjs b/skills/ns-download-asset/fetch-asset.cjs index b3090f2..b029004 100644 --- a/skills/ns-download-asset/fetch-asset.cjs +++ b/skills/ns-download-asset/fetch-asset.cjs @@ -31,6 +31,7 @@ const { randomUUID } = require('crypto') const { pipeline } = require('stream/promises') const http = require('http') const https = require('https') +const zlib = require('zlib') const EXTENSIONS = { cpuprofile: '.cpuprofile', @@ -433,7 +434,10 @@ async function downloadAsset (consoleUrl, token, assetId, destPath, validatedIps const req = transport.request(url, { headers: { 'x-nsolid-service-token': token, - Accept: 'application/json' + Accept: 'application/json', + // We handle gzip manually below; ask the server for identity so the + // common case streams straight to disk. + 'Accept-Encoding': 'identity' }, lookup }, resolve) @@ -451,8 +455,23 @@ async function downloadAsset (consoleUrl, token, assetId, destPath, validatedIps ) } + // The console can serve assets gzip-compressed regardless of + // Accept-Encoding negotiation (its `compressed` flag). Decompress before + // writing so the on-disk asset and the recorded fileSize are always the + // plain payload — fetch() used to do this transparently. + const encoding = String(res.headers['content-encoding'] ?? 'identity').toLowerCase() + if (encoding !== 'identity' && encoding !== 'gzip') { + res.resume() // drain the socket before throwing + throw new Error(`Unsupported Content-Encoding "${encoding}" for asset ${assetId}`) + } + abortInFlight = (error) => res.destroy(error) - await pipeline(res, fs.createWriteStream(tempPath, { flags: 'wx' })) + const writer = fs.createWriteStream(tempPath, { flags: 'wx' }) + if (encoding === 'gzip') { + await pipeline(res, zlib.createGunzip(), writer) + } else { + await pipeline(res, writer) + } fs.renameSync(tempPath, destPath) return fs.statSync(destPath).size diff --git a/skills/ns-generate-asset/fetch-asset.cjs b/skills/ns-generate-asset/fetch-asset.cjs index b3090f2..b029004 100644 --- a/skills/ns-generate-asset/fetch-asset.cjs +++ b/skills/ns-generate-asset/fetch-asset.cjs @@ -31,6 +31,7 @@ const { randomUUID } = require('crypto') const { pipeline } = require('stream/promises') const http = require('http') const https = require('https') +const zlib = require('zlib') const EXTENSIONS = { cpuprofile: '.cpuprofile', @@ -433,7 +434,10 @@ async function downloadAsset (consoleUrl, token, assetId, destPath, validatedIps const req = transport.request(url, { headers: { 'x-nsolid-service-token': token, - Accept: 'application/json' + Accept: 'application/json', + // We handle gzip manually below; ask the server for identity so the + // common case streams straight to disk. + 'Accept-Encoding': 'identity' }, lookup }, resolve) @@ -451,8 +455,23 @@ async function downloadAsset (consoleUrl, token, assetId, destPath, validatedIps ) } + // The console can serve assets gzip-compressed regardless of + // Accept-Encoding negotiation (its `compressed` flag). Decompress before + // writing so the on-disk asset and the recorded fileSize are always the + // plain payload — fetch() used to do this transparently. + const encoding = String(res.headers['content-encoding'] ?? 'identity').toLowerCase() + if (encoding !== 'identity' && encoding !== 'gzip') { + res.resume() // drain the socket before throwing + throw new Error(`Unsupported Content-Encoding "${encoding}" for asset ${assetId}`) + } + abortInFlight = (error) => res.destroy(error) - await pipeline(res, fs.createWriteStream(tempPath, { flags: 'wx' })) + const writer = fs.createWriteStream(tempPath, { flags: 'wx' }) + if (encoding === 'gzip') { + await pipeline(res, zlib.createGunzip(), writer) + } else { + await pipeline(res, writer) + } fs.renameSync(tempPath, destPath) return fs.statSync(destPath).size diff --git a/skills/ns-memory-spike-analysis/fetch-asset.cjs b/skills/ns-memory-spike-analysis/fetch-asset.cjs index b3090f2..b029004 100644 --- a/skills/ns-memory-spike-analysis/fetch-asset.cjs +++ b/skills/ns-memory-spike-analysis/fetch-asset.cjs @@ -31,6 +31,7 @@ const { randomUUID } = require('crypto') const { pipeline } = require('stream/promises') const http = require('http') const https = require('https') +const zlib = require('zlib') const EXTENSIONS = { cpuprofile: '.cpuprofile', @@ -433,7 +434,10 @@ async function downloadAsset (consoleUrl, token, assetId, destPath, validatedIps const req = transport.request(url, { headers: { 'x-nsolid-service-token': token, - Accept: 'application/json' + Accept: 'application/json', + // We handle gzip manually below; ask the server for identity so the + // common case streams straight to disk. + 'Accept-Encoding': 'identity' }, lookup }, resolve) @@ -451,8 +455,23 @@ async function downloadAsset (consoleUrl, token, assetId, destPath, validatedIps ) } + // The console can serve assets gzip-compressed regardless of + // Accept-Encoding negotiation (its `compressed` flag). Decompress before + // writing so the on-disk asset and the recorded fileSize are always the + // plain payload — fetch() used to do this transparently. + const encoding = String(res.headers['content-encoding'] ?? 'identity').toLowerCase() + if (encoding !== 'identity' && encoding !== 'gzip') { + res.resume() // drain the socket before throwing + throw new Error(`Unsupported Content-Encoding "${encoding}" for asset ${assetId}`) + } + abortInFlight = (error) => res.destroy(error) - await pipeline(res, fs.createWriteStream(tempPath, { flags: 'wx' })) + const writer = fs.createWriteStream(tempPath, { flags: 'wx' }) + if (encoding === 'gzip') { + await pipeline(res, zlib.createGunzip(), writer) + } else { + await pipeline(res, writer) + } fs.renameSync(tempPath, destPath) return fs.statSync(destPath).size From eed21c5d676e6510a66395ecad0df94a9581f98a Mon Sep 17 00:00:00 2001 From: Cesar-M-Diaz Date: Tue, 25 Aug 2026 13:38:20 +0200 Subject: [PATCH 6/8] fix(skills): remove track-heap-objects from leak hunter workflow asset-summary does not support heap-profile assets (the type produced by track-heap-objects), so Phase 3 of ns-advanced-memory-leak-hunter failed with 'Unsupported asset type' during baseline-vs-peak hunts. - Leak hunter now captures the peak with heap-sampling only; closure/retainer suspicion is handled by correlating allocator call stacks with runtime-code - Add guardrail documenting the unsupported asset type - Update ns-analyze-asset cross-reference (no longer recommends track-heap-objects) - Drop track-heap-objects mention from ns-memory-spike-analysis bundle description --- bundle.json | 2 +- packages/core/bundle.json | 2 +- skills/ns-advanced-memory-leak-hunter/SKILL.md | 5 +++-- skills/ns-analyze-asset/SKILL.md | 2 +- 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/bundle.json b/bundle.json index d313da8..6f5e138 100644 --- a/bundle.json +++ b/bundle.json @@ -72,7 +72,7 @@ { "name": "ns-memory-spike-analysis", "path": "skills/ns-memory-spike-analysis", - "description": "Analyze memory spikes and suspected leaks via baseline/peak heap sampling, with optional track-heap-objects for retainer analysis", + "description": "Analyze memory spikes and suspected leaks via baseline/peak heap sampling comparison", "requiresMcp": ["nsolid-console"] }, { diff --git a/packages/core/bundle.json b/packages/core/bundle.json index d313da8..6f5e138 100644 --- a/packages/core/bundle.json +++ b/packages/core/bundle.json @@ -72,7 +72,7 @@ { "name": "ns-memory-spike-analysis", "path": "skills/ns-memory-spike-analysis", - "description": "Analyze memory spikes and suspected leaks via baseline/peak heap sampling, with optional track-heap-objects for retainer analysis", + "description": "Analyze memory spikes and suspected leaks via baseline/peak heap sampling comparison", "requiresMcp": ["nsolid-console"] }, { diff --git a/skills/ns-advanced-memory-leak-hunter/SKILL.md b/skills/ns-advanced-memory-leak-hunter/SKILL.md index 63b0eb5..946e7e7 100644 --- a/skills/ns-advanced-memory-leak-hunter/SKILL.md +++ b/skills/ns-advanced-memory-leak-hunter/SKILL.md @@ -29,7 +29,7 @@ description: >- ### Phase 3: Capture the Peak / Leak State 1. Once you confirm memory has substantially grown from the baseline, trigger a second analysis. -2. Use advanced `track-heap-objects` only for closure/retainer suspicion; otherwise use `heap-sampling` for 60 seconds. Both require the resolved agent `id`. +2. Capture the peak with `heap-sampling` for 60 seconds. Requires the resolved agent `id`. Do not use `track-heap-objects`: the resulting `heap-profile` asset is not supported by `asset-summary`. When closure/retainer suspicion exists, correlate the peak sample's allocator call stacks with the source using `runtime-code` (Phase 5) instead. 3. Wait for the operation to complete: ``` node "/wait.cjs" 60 @@ -101,4 +101,5 @@ description: >- - **No early assumptions**: Never declare a memory leak from a single snapshot. Always compare baseline to peak. - **Reuse what exists**: Do not capture a new baseline or peak sample if the user already supplied the needed assets. - **Wait times**: Memory tools block the thread. Do not spam endpoints while an asset is in progress. -- Never use the MCP `asset` tool to download raw assets (still exposed by older console versions); always use the bundled `fetch-asset.cjs`. \ No newline at end of file +- Never use the MCP `asset` tool to download raw assets (still exposed by older console versions); always use the bundled `fetch-asset.cjs`. +- **Unsupported asset types**: `asset-summary` does not support `heap-profile` assets (`track-heap-objects`). Never capture them in this workflow — baseline and peak must always be heap samples. diff --git a/skills/ns-analyze-asset/SKILL.md b/skills/ns-analyze-asset/SKILL.md index e665850..6fba2ff 100644 --- a/skills/ns-analyze-asset/SKILL.md +++ b/skills/ns-analyze-asset/SKILL.md @@ -73,7 +73,7 @@ Inspect the summary for these signals: - Check `events-historic` with the same `app` or agent `id`, plus `start`/`end`, for near-OOM or process-blocked events at the snapshot time. **Follow-up recommendation:** -- If allocation stack traces are insufficient, recommend advanced `track-heap-objects` via `ns-advanced-memory-leak-hunter`. +- If allocation stack traces are insufficient, recommend a deeper leak hunt (baseline vs peak heap sampling) via `ns-advanced-memory-leak-hunter`. - For deeper leak hunting workflows, reference the `ns-advanced-memory-leak-hunter` skill. ### 4. Correlate with Runtime Context From 15ffedcd5c22e263c370c32233757cb3dae60a86 Mon Sep 17 00:00:00 2001 From: Cesar-M-Diaz Date: Thu, 27 Aug 2026 15:05:32 +0200 Subject: [PATCH 7/8] fix(skills): align heap-profile guidance with asset-summary type contract ns-generate-asset no longer routes heap-tracking captures to ns-analyze-asset or ns-advanced-memory-leak-hunter (heap-profile is not summarizable); the local .heapprofile from fetch-asset.cjs is the deliverable. ns-analyze-asset renames "Heap Profile or Heap Sample" to "Heap Sample" and adds a note that only heap samples can be summarized; heap-profile assets may only be downloaded locally via fetch-asset.cjs and never read raw into context. --- skills/ns-analyze-asset/SKILL.md | 3 ++- skills/ns-generate-asset/SKILL.md | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/skills/ns-analyze-asset/SKILL.md b/skills/ns-analyze-asset/SKILL.md index 6fba2ff..7e50d2d 100644 --- a/skills/ns-analyze-asset/SKILL.md +++ b/skills/ns-analyze-asset/SKILL.md @@ -45,7 +45,8 @@ Heap snapshot summarization is asynchronous and may not be ready on the first ca - Explain the hot path and the most expensive bottleneck. - Focus on user-owned code. If the top cost is in Node internals or `node_modules`, explain the nearest relevant user-owned caller instead. -#### Heap Profile or Heap Sample +#### Heap Sample +- Only heap samples (from `heap-sampling`, type `heap-sample`) can be summarized. `heap-profile` assets (from `track-heap-objects`) are not supported by `asset-summary`; they can only be downloaded locally via `fetch-asset.cjs` and must never be read raw into context. - Identify top allocating constructors by self size and retained size. - Call out suspicious allocation patterns (e.g. unusually large arrays, many short-lived objects of the same type). diff --git a/skills/ns-generate-asset/SKILL.md b/skills/ns-generate-asset/SKILL.md index e86f255..3a39aea 100644 --- a/skills/ns-generate-asset/SKILL.md +++ b/skills/ns-generate-asset/SKILL.md @@ -57,7 +57,7 @@ Run the bundled wait script (use the absolute path of the directory where you re - For capture-only requests, report asset type, asset ID, app name, agent ID, duration/thread ID, and local path. - If the user asked to analyze, summarize, interpret, or explain the captured asset, read and follow `../ns-analyze-asset/SKILL.md` after the asset is ready. Do not duplicate its analysis rules here. - Pass this handoff payload: `assetId`, `assetType` (`cpuprofile`, `heapprofile`, or `heapsnapshot`), `appName`, `agentId`, `threadId`, `duration`, and local path if downloaded. -- For heap tracking, report capture metadata and local path; only route to `ns-analyze-asset` if `asset-summary` supports the returned asset. Otherwise recommend `ns-advanced-memory-leak-hunter` for interpretation. +- For heap tracking, report capture metadata and local path. The resulting `heap-profile` asset cannot be summarized by `asset-summary`; do not route it to `ns-analyze-asset` or `ns-advanced-memory-leak-hunter`. The local `.heapprofile` saved via `fetch-asset.cjs` is the deliverable; inspect the raw file only if the user explicitly asks. ## Guardrails - Do not use `runtime-code` or `workspace_delta`. From fa27684e5ef94aa3a0ea6a550667c2ccc0bd33ec Mon Sep 17 00:00:00 2001 From: Cesar-M-Diaz Date: Thu, 27 Aug 2026 15:33:11 +0200 Subject: [PATCH 8/8] fix(skills): scope generate-asset analysis handoff to summarizable types MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The general analysis handoff routed every analyze/summarize request to ns-analyze-asset, conflicting with the heap-tracking exclusion below it. Narrow the handoff to CPU profile, heap sample, and heap snapshot — exactly the types asset-summary supports. --- skills/ns-generate-asset/SKILL.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/skills/ns-generate-asset/SKILL.md b/skills/ns-generate-asset/SKILL.md index 3a39aea..8fa8b29 100644 --- a/skills/ns-generate-asset/SKILL.md +++ b/skills/ns-generate-asset/SKILL.md @@ -55,7 +55,7 @@ Run the bundled wait script (use the absolute path of the directory where you re ### 7. Report or Hand Off for Analysis - For capture-only requests, report asset type, asset ID, app name, agent ID, duration/thread ID, and local path. -- If the user asked to analyze, summarize, interpret, or explain the captured asset, read and follow `../ns-analyze-asset/SKILL.md` after the asset is ready. Do not duplicate its analysis rules here. +- If the user asked to analyze, summarize, interpret, or explain a CPU profile, heap sample, or heap snapshot, read and follow `../ns-analyze-asset/SKILL.md` after the asset is ready. Do not duplicate its analysis rules here. - Pass this handoff payload: `assetId`, `assetType` (`cpuprofile`, `heapprofile`, or `heapsnapshot`), `appName`, `agentId`, `threadId`, `duration`, and local path if downloaded. - For heap tracking, report capture metadata and local path. The resulting `heap-profile` asset cannot be summarized by `asset-summary`; do not route it to `ns-analyze-asset` or `ns-advanced-memory-leak-hunter`. The local `.heapprofile` saved via `fetch-asset.cjs` is the deliverable; inspect the raw file only if the user explicitly asks.