diff --git a/pkg/npm/install.js b/pkg/npm/install.js index 9c86e9337..ada940f27 100644 --- a/pkg/npm/install.js +++ b/pkg/npm/install.js @@ -8,7 +8,7 @@ const crypto = require('crypto'); const fs = require('fs'); const path = require('path'); const os = require('os'); -const { execFileSync } = require('child_process'); +const { execFileSync, spawn } = require('child_process'); const { pipeline } = require('stream'); const REPO = 'DeusData/codebase-memory-mcp'; @@ -18,6 +18,20 @@ const MAX_REDIRECTS = 5; const DOWNLOAD_HOP_TIMEOUT_MS = 120_000; const CANDIDATE_TIMEOUT_MS = 15_000; const MAX_CHECKSUM_MANIFEST_BYTES = 1024 * 1024; +// Release archive resource limits. Keep in sync with the Go wrapper +// (pkg/go/cmd/codebase-memory-mcp/main.go) and the PyPI wrapper (_cli.py). +const MAX_RELEASE_ARCHIVE_BYTES = 256 * 1024 * 1024; +const MAX_ARCHIVE_MEMBERS = 64; +const MAX_ARCHIVE_MEMBER_BYTES = 256 * 1024 * 1024; +const MAX_ARCHIVE_EXPANDED_BYTES = 512 * 1024 * 1024; +const DEFAULT_ARCHIVE_LIMITS = Object.freeze({ + compressedBytes: MAX_RELEASE_ARCHIVE_BYTES, + members: MAX_ARCHIVE_MEMBERS, + memberBytes: MAX_ARCHIVE_MEMBER_BYTES, + expandedBytes: MAX_ARCHIVE_EXPANDED_BYTES, +}); +// Bytes of tar's own diagnostics kept for the failure message. +const MAX_TAR_STDERR_CHARS = 4096; const WINDOWS_BINARY_NAME = 'codebase-memory-mcp.exe'; const UNIX_ARCHIVE_NAMES = [ 'codebase-memory-mcp', @@ -90,21 +104,131 @@ function validateExactTarMemberListing(listing, expectedNames) { } } -function extractExactTarArchive( - archivePath, destPath, expectedNames, targetName, runFile = execFileSync, +function validateArchiveLimits(limits) { + if (!limits || !(limits.compressedBytes > 0) || !(limits.members > 0) || + !(limits.memberBytes > 0) || !(limits.expandedBytes > 0)) { + throw new Error('invalid archive resource safety limits'); + } +} + +function requireCompressedArchiveWithinLimit(archivePath, limits) { + const status = fs.statSync(archivePath); + if (!status.isFile()) { + throw new Error('release archive is not a regular file'); + } + if (status.size > limits.compressedBytes) { + throw new Error( + `release archive exceeds the ${limits.compressedBytes}-byte compressed safety limit`, + ); + } +} + +// Streams one member through `tar -xzOf` into a file this call creates, +// counting every byte against the per-member limit. The file is removed only +// when this call created it: a pre-existing target fails untouched. +function extractTarMemberWithLimit( + archivePath, target, memberName, limits, spawnFile = spawn, ) { + return new Promise((resolve, reject) => { + let fd; + try { + fd = fs.openSync(target, 'wx', 0o600); + } catch (err) { + reject(err); + return; + } + let written = 0; + let failure = null; + let settled = false; + let stderr = ''; + const finish = (err) => { + if (settled) return; + settled = true; + try { fs.closeSync(fd); } catch (closeError) { err = err || closeError; } + if (err) { + try { fs.unlinkSync(target); } catch (_) { /* already gone */ } + reject(err); + return; + } + resolve(written); + }; + const child = spawnFile('tar', ['-xzOf', archivePath, memberName], { + stdio: ['ignore', 'pipe', 'pipe'], + windowsHide: true, + }); + const fail = (err) => { + if (failure) return; + failure = err; + child.kill(); + }; + child.stdout.on('data', (chunk) => { + if (failure) return; + written += chunk.length; + if (written > limits.memberBytes) { + fail(new Error( + `archive member ${JSON.stringify(memberName)} exceeds the ${limits.memberBytes}-byte expanded safety limit`, + )); + return; + } + try { + let offset = 0; + while (offset < chunk.length) { + offset += fs.writeSync(fd, chunk, offset, chunk.length - offset); + } + } catch (err) { + fail(err); + } + }); + child.stderr.setEncoding('utf8'); + child.stderr.on('data', (text) => { + if (stderr.length < MAX_TAR_STDERR_CHARS) stderr += text; + }); + child.on('error', (err) => { + failure = failure || err; + // A process that never started (no tar on PATH) has no exit to wait for. + if (child.pid === undefined) finish(failure); + }); + child.on('close', (code, signal) => { + const detail = stderr.trim(); + let err = failure; + if (!err && signal) { + err = new Error( + `tar was terminated by ${signal} while extracting ${memberName}: ${detail}`, + ); + } else if (!err && code !== 0) { + err = new Error( + `tar exited with status ${code} while extracting ${memberName}: ${detail}`, + ); + } else if (!err && written === 0) { + err = new Error(`archive member ${memberName} is empty`); + } + finish(err); + }); + }); +} + +async function extractExactTarArchive( + archivePath, destPath, expectedNames, targetName, + runFile = execFileSync, limits = DEFAULT_ARCHIVE_LIMITS, spawnFile = spawn, +) { + validateArchiveLimits(limits); + requireCompressedArchiveWithinLimit(archivePath, limits); + // The exact root namespace is the member bound: its four names lie well + // within MAX_ARCHIVE_MEMBERS, which the zip path enforces by count. + if (expectedNames.length > limits.members) { + throw new Error(`archive namespace exceeds the ${limits.members}-member safety limit`); + } const listing = runFile('tar', ['-tzf', archivePath], { encoding: 'utf8', maxBuffer: MAX_CHECKSUM_MANIFEST_BYTES, windowsHide: true, }); validateExactTarMemberListing(listing, expectedNames); - // Extract only the fixed root executable. Even a malformed tar implementation - // cannot write an unvalidated companion member outside the private temp tree. - runFile('tar', ['-xzf', archivePath, '-C', destPath, targetName], { - stdio: 'inherit', - windowsHide: true, - }); + // Only the fixed root executable is ever written, through the counted + // writer; the companions are listed but never extracted. + const target = path.join(destPath, targetName); + await extractTarMemberWithLimit(archivePath, target, targetName, limits, spawnFile); + fs.chmodSync(target, 0o755); } function downloadHop(url, dest, maxBytes) { @@ -123,7 +247,11 @@ function downloadHop(url, dest, maxBytes) { const req = https.get(url, (res) => { response = res; - res.on('error', (err) => finish(err)); + let piped = false; + // Before the body is piped, a response error ends the hop directly; + // afterwards the pipeline reports it, so the partial file is discarded + // before the hop settles. + res.on('error', (err) => { if (!piped) finish(err); }); const redirectCodes = new Set([301, 302, 303, 307, 308]); if (redirectCodes.has(res.statusCode)) { const location = res.headers.location; @@ -149,6 +277,12 @@ function downloadHop(url, dest, maxBytes) { finish(new Error(`HTTP ${res.statusCode} for ${url}`)); return; } + const declared = Number(res.headers['content-length']); + if (maxBytes && Number.isFinite(declared) && declared > maxBytes) { + res.destroy(); + finish(new Error(`Download exceeds the ${maxBytes}-byte safety limit`)); + return; + } let received = 0; res.on('data', (chunk) => { @@ -158,8 +292,20 @@ function downloadHop(url, dest, maxBytes) { } }); const file = fs.createWriteStream(dest, { flags: 'w' }); + piped = true; pipeline(res, file, (err) => { - finish(err, { redirect: null }); + if (!err) { + finish(null, { redirect: null }); + return; + } + // Discard the partial file once its handle is closed (Windows refuses + // to unlink an open file), then report the failure. + const discard = () => { + try { fs.unlinkSync(dest); } catch (_) { /* nothing was written */ } + finish(err); + }; + if (file.closed) discard(); + else file.once('close', discard); }); }); req.on('error', (err) => finish(err)); @@ -457,19 +603,37 @@ function installWindowsBinaryAtomically(sourceDir, destDir, verifier = verifyCan } } -function extractZipOnWindows(archivePath, destPath, requiredNames, extractNames) { +function extractZipOnWindows( + archivePath, destPath, requiredNames, extractNames, + limits = DEFAULT_ARCHIVE_LIMITS, shell = 'powershell', +) { + validateArchiveLimits(limits); + requireCompressedArchiveWithinLimit(archivePath, limits); // -EncodedCommand is a constant program. Paths travel only through the child // environment and are consumed with -LiteralPath, so PowerShell never parses // user/TEMP path bytes as source code or wildcard syntax. const script = [ "$ErrorActionPreference = 'Stop'", 'Add-Type -AssemblyName System.IO.Compression.FileSystem', + '$maxMembers = [int]$env:CBM_NPM_MAX_MEMBERS', + '$maxMemberBytes = [long]$env:CBM_NPM_MAX_MEMBER_BYTES', + '$maxExpandedBytes = [long]$env:CBM_NPM_MAX_EXPANDED_BYTES', + '$memberCount = 0', + '$declaredExpanded = [long]0', '$zip = [System.IO.Compression.ZipFile]::OpenRead($env:CBM_NPM_ARCHIVE_PATH)', "$seen = New-Object 'System.Collections.Generic.HashSet[string]' ([System.StringComparer]::OrdinalIgnoreCase)", "$requiredNames = @($env:CBM_NPM_REQUIRED_NAMES.Split('|'))", '$targetCounts = @{}', 'foreach ($requiredName in $requiredNames) { $targetCounts[$requiredName] = 0 }', 'try { foreach ($entry in $zip.Entries) { ' + + '$memberCount = $memberCount + 1; ' + + 'if ($memberCount -gt $maxMembers) { ' + + 'throw "archive exceeds the $maxMembers-member safety limit" }; ' + + 'if ($entry.Length -gt $maxMemberBytes) { ' + + 'throw "archive member $($entry.FullName) exceeds the $maxMemberBytes-byte expanded safety limit" }; ' + + 'if ($declaredExpanded -gt ($maxExpandedBytes - $entry.Length)) { ' + + 'throw "archive exceeds the $maxExpandedBytes-byte aggregate expanded safety limit" }; ' + + '$declaredExpanded = $declaredExpanded + $entry.Length; ' + "$name = $entry.FullName.Replace('\\', '/'); " + "$directory = $name.EndsWith('/'); " + "$segmentsPath = if ($directory) { $name.TrimEnd('/') } else { $name }; " + @@ -491,27 +655,61 @@ function extractZipOnWindows(archivePath, destPath, requiredNames, extractNames) 'if ($seen.Count -ne $requiredNames.Count) { ' + 'throw "archive does not match the exact release root-file allowlist" }', "$extractNames = @($env:CBM_NPM_EXTRACT_NAMES.Split('|'))", + '$actualExpanded = [long]0', + '$buffer = New-Object byte[] 65536', '$extractZip = [System.IO.Compression.ZipFile]::OpenRead($env:CBM_NPM_ARCHIVE_PATH)', + // Copy by hand with a byte counter: ExtractToFile would write whatever the + // compressed stream yields, regardless of the declared size. 'try { foreach ($extractName in $extractNames) { ' + '$entry = @($extractZip.Entries | Where-Object { $_.FullName -ceq $extractName })[0]; ' + - '[System.IO.Compression.ZipFileExtensions]::ExtractToFile(' + - '$entry, (Join-Path $env:CBM_NPM_DEST_PATH $extractName), $false) ' + + '$targetPath = Join-Path $env:CBM_NPM_DEST_PATH $extractName; ' + + '$remaining = $maxExpandedBytes - $actualExpanded; ' + + '$copied = [long]0; ' + + '$source = $entry.Open(); ' + + '$target = [System.IO.File]::Open($targetPath, [System.IO.FileMode]::CreateNew, [System.IO.FileAccess]::Write); ' + + 'try { while ($true) { ' + + '$count = $source.Read($buffer, 0, $buffer.Length); ' + + 'if ($count -le 0) { break }; ' + + '$copied = $copied + $count; ' + + 'if ($copied -gt $maxMemberBytes) { ' + + 'throw "archive member $extractName exceeds the $maxMemberBytes-byte actual expanded safety limit" }; ' + + 'if ($copied -gt $remaining) { ' + + 'throw "archive exceeds the $maxExpandedBytes-byte aggregate actual expanded safety limit" }; ' + + '$target.Write($buffer, 0, $count) } ' + + '} catch { $target.Dispose(); ' + + 'Remove-Item -LiteralPath $targetPath -Force -ErrorAction SilentlyContinue; throw ' + + '} finally { $target.Dispose(); $source.Dispose() }; ' + + 'if ($copied -ne $entry.Length) { ' + + 'Remove-Item -LiteralPath $targetPath -Force -ErrorAction SilentlyContinue; ' + + 'throw "archive member $extractName actual size $copied does not match declared size $($entry.Length)" }; ' + + '$actualExpanded = $actualExpanded + $copied ' + '} } finally { $extractZip.Dispose() }', ].join('; '); const encoded = Buffer.from(script, 'utf16le').toString('base64'); - execFileSync('powershell', [ - '-NoLogo', '-NoProfile', '-NonInteractive', '-EncodedCommand', encoded, - ], { - env: { - ...process.env, - CBM_NPM_ARCHIVE_PATH: archivePath, - CBM_NPM_DEST_PATH: destPath, - CBM_NPM_REQUIRED_NAMES: requiredNames.join('|'), - CBM_NPM_EXTRACT_NAMES: extractNames.join('|'), - }, - stdio: 'inherit', - windowsHide: true, - }); + try { + execFileSync(shell, [ + '-NoLogo', '-NoProfile', '-NonInteractive', '-EncodedCommand', encoded, + ], { + env: { + ...process.env, + CBM_NPM_ARCHIVE_PATH: archivePath, + CBM_NPM_DEST_PATH: destPath, + CBM_NPM_REQUIRED_NAMES: requiredNames.join('|'), + CBM_NPM_EXTRACT_NAMES: extractNames.join('|'), + CBM_NPM_MAX_MEMBERS: String(limits.members), + CBM_NPM_MAX_MEMBER_BYTES: String(limits.memberBytes), + CBM_NPM_MAX_EXPANDED_BYTES: String(limits.expandedBytes), + }, + encoding: 'utf8', + stdio: ['ignore', 'inherit', 'pipe'], + windowsHide: true, + }); + } catch (err) { + // Surface the script's own reason (for example which limit was hit) + // instead of a bare "Command failed". + const detail = err && err.stderr ? String(err.stderr).trim() : ''; + throw new Error(detail ? `zip extraction failed: ${detail}` : err.message); + } } // Fetch checksums.txt and verify the archive hash. @@ -592,12 +790,12 @@ async function main() { const tmpArchive = path.join(tmpDir, `cbm.${ext}`); try { - await download(url, tmpArchive); + await download(url, tmpArchive, MAX_RELEASE_ARCHIVE_BYTES); await verifyChecksum(tmpArchive, archive); // Validate the complete archive namespace, then extract only executables. if (ext === 'tar.gz') { - extractExactTarArchive( + await extractExactTarArchive( tmpArchive, tmpDir, archiveNames, binName, ); } else { @@ -674,9 +872,13 @@ if (require.main === module || module.parent == null) { } module.exports = { + DEFAULT_ARCHIVE_LIMITS, UNIX_ARCHIVE_NAMES, + WINDOWS_ARCHIVE_NAMES, WINDOWS_BINARY_NAME, + download, extractExactTarArchive, + extractZipOnWindows, installWindowsBinaryAtomically, validateExactTarMemberListing, windowsBinaryReady, diff --git a/pkg/npm/test/archive-fixtures.js b/pkg/npm/test/archive-fixtures.js new file mode 100644 index 000000000..5b28a2e91 --- /dev/null +++ b/pkg/npm/test/archive-fixtures.js @@ -0,0 +1,113 @@ +'use strict'; +// Small synthetic release archives for the wrapper tests. + +const { execFileSync } = require('node:child_process'); +const fs = require('node:fs'); +const path = require('node:path'); + +// Deterministic, poorly compressible bytes (xorshift32), so a fixture of a +// few hundred KiB still spans several pipe chunks after gzip. +function patternBytes(length, seed = 0x9e3779b9) { + const out = Buffer.alloc(length); + let state = seed >>> 0; + for (let index = 0; index < length; index += 1) { + state ^= state << 13; + state >>>= 0; + state ^= state >>> 17; + state ^= state << 5; + state >>>= 0; + out[index] = state & 0xff; + } + return out; +} + +// Writes entries ({ name, data }) into a staging directory under `root` and +// packs them with the system tar, the producer the release itself uses. +function writeTarGz(root, archiveName, entries) { + const stage = fs.mkdtempSync(path.join(root, 'stage-')); + for (const entry of entries) { + fs.writeFileSync(path.join(stage, entry.name), entry.data); + } + const archive = path.join(root, archiveName); + execFileSync( + 'tar', + ['-czf', archive, '-C', stage, ...entries.map((entry) => entry.name)], + { + env: { ...process.env, COPYFILE_DISABLE: '1' }, + stdio: ['ignore', 'ignore', 'inherit'], + windowsHide: true, + }, + ); + return archive; +} + +const CRC_TABLE = (() => { + const table = new Int32Array(256); + for (let n = 0; n < 256; n += 1) { + let c = n; + for (let k = 0; k < 8; k += 1) { + c = c & 1 ? 0xedb88320 ^ (c >>> 1) : c >>> 1; + } + table[n] = c; + } + return table; +})(); + +function crc32(buffer) { + let crc = -1; + for (const byte of buffer) { + crc = CRC_TABLE[(crc ^ byte) & 0xff] ^ (crc >>> 8); + } + return (crc ^ -1) >>> 0; +} + +// entries: [{ name, data, declaredSize = data.length }]; stored (no +// compression). Hand-rolled so the directory can claim a size that differs +// from the bytes the entry carries, which no zip writer would produce. +function buildZip(entries) { + const locals = []; + const centrals = []; + let offset = 0; + for (const entry of entries) { + const data = Buffer.isBuffer(entry.data) ? entry.data : Buffer.from(entry.data); + const name = Buffer.from(entry.name, 'utf8'); + const declaredSize = entry.declaredSize === undefined + ? data.length + : entry.declaredSize; + const crc = crc32(data); + + const local = Buffer.alloc(30 + name.length); + local.writeUInt32LE(0x04034b50, 0); + local.writeUInt16LE(20, 4); + local.writeUInt32LE(crc, 14); + local.writeUInt32LE(data.length, 18); + local.writeUInt32LE(declaredSize, 22); + local.writeUInt16LE(name.length, 26); + name.copy(local, 30); + + const central = Buffer.alloc(46 + name.length); + central.writeUInt32LE(0x02014b50, 0); + central.writeUInt16LE(20, 4); + central.writeUInt16LE(20, 6); + central.writeUInt32LE(crc, 16); + central.writeUInt32LE(data.length, 20); + central.writeUInt32LE(declaredSize, 24); + central.writeUInt16LE(name.length, 28); + central.writeUInt32LE(offset, 42); + name.copy(central, 46); + + locals.push(local, data); + centrals.push(central); + offset += local.length + data.length; + } + const directory = Buffer.concat(centrals); + const end = Buffer.alloc(22); + end.writeUInt32LE(0x06054b50, 0); + end.writeUInt16LE(entries.length, 8); + end.writeUInt16LE(entries.length, 10); + end.writeUInt32LE(directory.length, 12); + end.writeUInt32LE(offset, 16); + return Buffer.concat([...locals, directory, end]); +} + +module.exports = { buildZip, patternBytes, writeTarGz }; diff --git a/pkg/npm/test/archive-limits.test.js b/pkg/npm/test/archive-limits.test.js new file mode 100644 index 000000000..3aa786954 --- /dev/null +++ b/pkg/npm/test/archive-limits.test.js @@ -0,0 +1,348 @@ +'use strict'; + +const assert = require('node:assert/strict'); +const { EventEmitter } = require('node:events'); +const fs = require('node:fs'); +const os = require('node:os'); +const path = require('node:path'); +const { Readable } = require('node:stream'); +const test = require('node:test'); +const vm = require('node:vm'); + +const { + DEFAULT_ARCHIVE_LIMITS, + UNIX_ARCHIVE_NAMES, + WINDOWS_ARCHIVE_NAMES, + WINDOWS_BINARY_NAME, + extractExactTarArchive, + extractZipOnWindows, +} = require('../install.js'); +const { buildZip, patternBytes, writeTarGz } = require('./archive-fixtures.js'); + +const EXECUTABLE = 'codebase-memory-mcp'; +// Spans several pipe chunks, so an over-limit member is cut off mid-stream. +const LARGE_EXECUTABLE = patternBytes(300 * 1024); + +function withScratch(callback) { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'cbm-npm-archive-test-')); + return Promise.resolve() + .then(() => callback(root)) + .finally(() => fs.rmSync(root, { recursive: true, force: true })); +} + +function limitsWith(overrides) { + return { ...DEFAULT_ARCHIVE_LIMITS, ...overrides }; +} + +function releaseEntries(names, executableData) { + return names.map((name) => ({ + name, + data: executableData !== undefined && (name === EXECUTABLE || name === WINDOWS_BINARY_NAME) + ? executableData + : `payload of ${name}`, + })); +} + +function makeDestination(root) { + const destination = path.join(root, 'extract'); + fs.mkdirSync(destination); + return destination; +} + +// Stands in for the system tar listing where the archive itself is damaged +// on purpose and the real listing would fail before the writer runs. +function fakeTarListing(names) { + return (command, args) => { + assert.equal(command, 'tar'); + assert.equal(args[0], '-tzf'); + return `${names.join('\n')}\n`; + }; +} + +function neverLists() { + throw new Error('the listing must not run'); +} + +test('tar extraction refuses an archive larger than the compressed limit', () => + withScratch(async (root) => { + const archive = writeTarGz(root, 'release.tar.gz', releaseEntries(UNIX_ARCHIVE_NAMES)); + const destination = makeDestination(root); + + await assert.rejects( + extractExactTarArchive( + archive, destination, UNIX_ARCHIVE_NAMES, EXECUTABLE, + neverLists, limitsWith({ compressedBytes: 16 }), + ), + /16-byte compressed safety limit/, + ); + assert.deepEqual(fs.readdirSync(destination), []); + })); + +test('tar extraction refuses an exact namespace wider than the member limit', () => + withScratch(async (root) => { + const archive = writeTarGz(root, 'release.tar.gz', releaseEntries(UNIX_ARCHIVE_NAMES)); + const destination = makeDestination(root); + + await assert.rejects( + extractExactTarArchive( + archive, destination, UNIX_ARCHIVE_NAMES, EXECUTABLE, + neverLists, limitsWith({ members: 2 }), + ), + /2-member safety limit/, + ); + assert.deepEqual(fs.readdirSync(destination), []); + })); + +test('tar extraction stops an over-limit member in the counted writer and leaves no file', () => + withScratch(async (root) => { + const archive = writeTarGz( + root, 'release.tar.gz', releaseEntries(UNIX_ARCHIVE_NAMES, LARGE_EXECUTABLE), + ); + const destination = makeDestination(root); + + // The real listing passes (names only); the limit trips while tar is + // still streaming the executable into the writer. + await assert.rejects( + extractExactTarArchive( + archive, destination, UNIX_ARCHIVE_NAMES, EXECUTABLE, + undefined, limitsWith({ memberBytes: 64 * 1024 }), + ), + /"codebase-memory-mcp" exceeds the 65536-byte expanded safety limit/, + ); + assert.deepEqual(fs.readdirSync(destination), []); + })); + +test('tar extraction leaves a pre-existing target untouched', () => + withScratch(async (root) => { + const archive = writeTarGz(root, 'release.tar.gz', releaseEntries(UNIX_ARCHIVE_NAMES)); + const destination = makeDestination(root); + const target = path.join(destination, EXECUTABLE); + fs.writeFileSync(target, 'keep'); + + await assert.rejects( + extractExactTarArchive(archive, destination, UNIX_ARCHIVE_NAMES, EXECUTABLE), + /EEXIST/, + ); + assert.equal(fs.readFileSync(target, 'utf8'), 'keep'); + })); + +test('tar extraction leaves no file when tar fails mid-stream', () => + withScratch(async (root) => { + const complete = writeTarGz( + root, 'complete.tar.gz', releaseEntries(UNIX_ARCHIVE_NAMES, LARGE_EXECUTABLE), + ); + const bytes = fs.readFileSync(complete); + const archive = path.join(root, 'truncated.tar.gz'); + fs.writeFileSync(archive, bytes.subarray(0, Math.floor(bytes.length * 0.6))); + const destination = makeDestination(root); + + await assert.rejects( + extractExactTarArchive( + archive, destination, UNIX_ARCHIVE_NAMES, EXECUTABLE, + fakeTarListing(UNIX_ARCHIVE_NAMES), + ), + // Non-zero exit, and tar's own diagnostic text after the colon. + /tar exited with status [1-9]\d* while extracting codebase-memory-mcp: \S/, + ); + assert.deepEqual(fs.readdirSync(destination), []); + })); + +test('tar extraction rejects an empty executable', () => + withScratch(async (root) => { + const archive = writeTarGz( + root, 'release.tar.gz', releaseEntries(UNIX_ARCHIVE_NAMES, Buffer.alloc(0)), + ); + const destination = makeDestination(root); + + await assert.rejects( + extractExactTarArchive(archive, destination, UNIX_ARCHIVE_NAMES, EXECUTABLE), + /archive member codebase-memory-mcp is empty/, + ); + assert.deepEqual(fs.readdirSync(destination), []); + })); + +test('tar extraction installs the executable byte-identical with the executable bit set', () => + withScratch(async (root) => { + const archive = writeTarGz( + root, 'release.tar.gz', releaseEntries(UNIX_ARCHIVE_NAMES, LARGE_EXECUTABLE), + ); + const destination = makeDestination(root); + + await extractExactTarArchive(archive, destination, UNIX_ARCHIVE_NAMES, EXECUTABLE); + + const target = path.join(destination, EXECUTABLE); + assert.ok(fs.readFileSync(target).equals(LARGE_EXECUTABLE)); + if (process.platform !== 'win32') { + // Windows has no mode bits; everywhere else the file must be runnable. + assert.equal(fs.statSync(target).mode & 0o111, 0o111); + } + assert.deepEqual(fs.readdirSync(destination), [EXECUTABLE]); + })); + +function loadInstallerWithHttps(fakeHttps) { + const installPath = path.join(__dirname, '..', 'install.js'); + const source = fs.readFileSync(installPath, 'utf8'); + const sandbox = { + Buffer, + URL, + __dirname: path.dirname(installPath), + __filename: installPath, + clearTimeout, + console, + exports: {}, + // A parent keeps the postinstall entry point from running on load. + module: { exports: {}, parent: {} }, + process, + require: (specifier) => { + if (specifier === './package.json') return { version: '0.0.0-test' }; + if (specifier === 'https') return fakeHttps; + return require(specifier); + }, + setTimeout, + }; + vm.runInNewContext(source, sandbox, { filename: installPath }); + return sandbox.module.exports; +} + +// Answers every request with one HTTP 200 response shaped by `configure`. +function fakeHttpsServing(configure) { + return { + get(url, callback) { + const request = new EventEmitter(); + request.destroy = (err) => { if (err) request.emit('error', err); }; + setImmediate(() => { + const response = new Readable({ read() {} }); + response.statusCode = 200; + response.headers = {}; + configure(response); + callback(response); + }); + return request; + }, + }; +} + +test('download refuses a response whose declared length exceeds the limit', () => + withScratch(async (root) => { + const { download } = loadInstallerWithHttps(fakeHttpsServing((response) => { + response.headers['content-length'] = '9'; + response.push(null); + })); + const destination = path.join(root, 'release.tar.gz'); + + await assert.rejects( + download('https://example.invalid/release.tar.gz', destination, 8), + /8-byte safety limit/, + ); + assert.equal(fs.existsSync(destination), false); + })); + +test('download refuses a body that exceeds the limit and removes the partial file', () => + withScratch(async (root) => { + const { download } = loadInstallerWithHttps(fakeHttpsServing((response) => { + response.push(Buffer.alloc(1024, 0x78)); + response.push(null); + })); + const destination = path.join(root, 'release.tar.gz'); + + await assert.rejects( + download('https://example.invalid/release.tar.gz', destination, 8), + /8-byte safety limit/, + ); + assert.equal(fs.existsSync(destination), false); + })); + +test('download within the limit stores the body', () => + withScratch(async (root) => { + const { download } = loadInstallerWithHttps(fakeHttpsServing((response) => { + response.headers['content-length'] = '7'; + response.push('payload'); + response.push(null); + })); + const destination = path.join(root, 'release.tar.gz'); + + await download('https://example.invalid/release.tar.gz', destination, 1024); + + assert.equal(fs.readFileSync(destination, 'utf8'), 'payload'); + })); + +// The Windows path runs a constant PowerShell program through powershell.exe, +// which only a Windows host provides (pwsh is not a wrapper dependency, and +// no PowerShell is installed on the Unix wrapper hosts), so these cases run +// in the Windows wrapper leg and are skipped elsewhere. +const WINDOWS_ONLY = process.platform === 'win32' + ? false + : 'zip extraction runs through powershell.exe, which this host does not provide'; + +test('Windows zip extraction refuses an archive with more members than the limit', { skip: WINDOWS_ONLY }, () => + withScratch(async (root) => { + const archive = path.join(root, 'members.zip'); + fs.writeFileSync(archive, buildZip([ + { name: 'one', data: 'a' }, + { name: 'two', data: 'b' }, + { name: 'three', data: 'c' }, + ])); + const destination = makeDestination(root); + + assert.throws( + () => extractZipOnWindows( + archive, destination, ['one', 'two', 'three'], ['one'], + limitsWith({ members: 2 }), + ), + /2-member safety limit/, + ); + assert.deepEqual(fs.readdirSync(destination), []); + })); + +test('Windows zip extraction refuses a member whose declared size exceeds the limit', { skip: WINDOWS_ONLY }, () => + withScratch(async (root) => { + const archive = path.join(root, 'member.zip'); + fs.writeFileSync(archive, buildZip([ + { name: WINDOWS_BINARY_NAME, data: Buffer.alloc(65, 0x78) }, + ])); + const destination = makeDestination(root); + + assert.throws( + () => extractZipOnWindows( + archive, destination, [WINDOWS_BINARY_NAME], [WINDOWS_BINARY_NAME], + limitsWith({ memberBytes: 64 }), + ), + /64-byte expanded safety limit/, + ); + assert.deepEqual(fs.readdirSync(destination), []); + })); + +test('Windows zip extraction stops a member that yields more bytes than it declares', { skip: WINDOWS_ONLY }, () => + withScratch(async (root) => { + const archive = path.join(root, 'longer.zip'); + fs.writeFileSync(archive, buildZip([ + { name: WINDOWS_BINARY_NAME, data: Buffer.alloc(600, 0x78), declaredSize: 10 }, + ])); + const destination = makeDestination(root); + + assert.throws( + () => extractZipOnWindows( + archive, destination, [WINDOWS_BINARY_NAME], [WINDOWS_BINARY_NAME], + limitsWith({ memberBytes: 64 }), + ), + /64-byte actual expanded safety limit/, + ); + assert.deepEqual(fs.readdirSync(destination), []); + })); + +test('Windows zip extraction within the limits installs only the executable', { skip: WINDOWS_ONLY }, () => + withScratch(async (root) => { + const archive = path.join(root, 'release.zip'); + fs.writeFileSync(archive, buildZip(releaseEntries(WINDOWS_ARCHIVE_NAMES))); + const destination = makeDestination(root); + + extractZipOnWindows( + archive, destination, WINDOWS_ARCHIVE_NAMES, [WINDOWS_BINARY_NAME], + ); + + assert.equal( + fs.readFileSync(path.join(destination, WINDOWS_BINARY_NAME), 'utf8'), + `payload of ${WINDOWS_BINARY_NAME}`, + ); + assert.deepEqual(fs.readdirSync(destination), [WINDOWS_BINARY_NAME]); + })); diff --git a/pkg/npm/test/package-publication.test.js b/pkg/npm/test/package-publication.test.js index 560aec700..258bdd74b 100644 --- a/pkg/npm/test/package-publication.test.js +++ b/pkg/npm/test/package-publication.test.js @@ -13,6 +13,7 @@ const { installWindowsBinaryAtomically, validateExactTarMemberListing, } = require('../install.js'); +const { writeTarGz } = require('./archive-fixtures.js'); function exactUnixListing(extra = []) { return [...UNIX_ARCHIVE_NAMES, ...extra].join('\n') + '\n'; @@ -33,23 +34,37 @@ test('Unix archive validation rejects traversal and unexpected members', () => { ); }); -test('Unix extraction requests only the validated root executable', () => { - const calls = []; - const runner = (command, args) => { - calls.push({ command, args: [...args] }); - return calls.length === 1 ? exactUnixListing() : Buffer.alloc(0); - }; +test('Unix extraction lists through the system tar and writes only the root executable', async () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'cbm-npm-extract-test-')); + try { + const archive = writeTarGz( + root, 'release.tar.gz', + UNIX_ARCHIVE_NAMES.map((name) => ({ name, data: `member:${name}` })), + ); + const destination = path.join(root, 'extract'); + fs.mkdirSync(destination); + const calls = []; + const runner = (command, args) => { + calls.push({ command, args: [...args] }); + return exactUnixListing(); + }; - extractExactTarArchive( - '/tmp/release.tar.gz', '/tmp/extract', UNIX_ARCHIVE_NAMES, - 'codebase-memory-mcp', runner, - ); + await extractExactTarArchive( + archive, destination, UNIX_ARCHIVE_NAMES, 'codebase-memory-mcp', runner, + ); - assert.deepEqual(calls[0].args, ['-tzf', '/tmp/release.tar.gz']); - assert.deepEqual( - calls[1].args, - ['-xzf', '/tmp/release.tar.gz', '-C', '/tmp/extract', 'codebase-memory-mcp'], - ); + // The listing runner sees the namespace query only; the executable is + // written by the counted `tar -xzOf` writer, never by `tar -x` into the + // tree, so the companions never touch the disk. + assert.deepEqual(calls.map((call) => call.args), [['-tzf', archive]]); + assert.equal( + fs.readFileSync(path.join(destination, 'codebase-memory-mcp'), 'utf8'), + 'member:codebase-memory-mcp', + ); + assert.deepEqual(fs.readdirSync(destination), ['codebase-memory-mcp']); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } }); function writeBinary(directory, tag) { diff --git a/pkg/pypi/src/codebase_memory_mcp/_cli.py b/pkg/pypi/src/codebase_memory_mcp/_cli.py index 458b8c350..9faccc005 100644 --- a/pkg/pypi/src/codebase_memory_mcp/_cli.py +++ b/pkg/pypi/src/codebase_memory_mcp/_cli.py @@ -1,5 +1,6 @@ """Downloads codebase-memory-mcp on first run, then runs its native entry point.""" +import collections import errno import hashlib import json @@ -42,6 +43,22 @@ _MAX_REDIRECTS = 5 _NETWORK_TIMEOUT_SECONDS = 120 _CANDIDATE_TIMEOUT_SECONDS = 15 _MAX_CHECKSUM_MANIFEST_BYTES = 1024 * 1024 +# Release archive resource limits. Keep in sync with the Go wrapper +# (pkg/go/cmd/codebase-memory-mcp/main.go) and the npm wrapper (install.js). +_MAX_RELEASE_ARCHIVE_BYTES = 256 * 1024 * 1024 +_MAX_ARCHIVE_MEMBERS = 64 +_MAX_ARCHIVE_MEMBER_BYTES = 256 * 1024 * 1024 +_MAX_ARCHIVE_EXPANDED_BYTES = 512 * 1024 * 1024 +_ARCHIVE_COPY_CHUNK_BYTES = 64 * 1024 +_ArchiveLimits = collections.namedtuple( + "_ArchiveLimits", "compressed_bytes members member_bytes expanded_bytes" +) +_DEFAULT_ARCHIVE_LIMITS = _ArchiveLimits( + compressed_bytes=_MAX_RELEASE_ARCHIVE_BYTES, + members=_MAX_ARCHIVE_MEMBERS, + member_bytes=_MAX_ARCHIVE_MEMBER_BYTES, + expanded_bytes=_MAX_ARCHIVE_EXPANDED_BYTES, +) _REDIRECT_CODES = frozenset({301, 302, 303, 307, 308}) _RUNTIME_LOCK_NAME = ".codebase-memory-mcp-runtime.lock" _RUNTIME_LOCK_WAIT_SECONDS = 45 @@ -113,27 +130,50 @@ def _download_https(url: str, dest: str, max_bytes: int = 0) -> None: with response: _validate_url_scheme(response.geturl()) + declared = (response.headers.get("Content-Length") or "").strip() + # Only plain ASCII digits are a length (str.isdigit also accepts + # characters that int() rejects); anything else is left to the + # byte counter below. + if max_bytes and re.fullmatch(r"[0-9]+", declared): + if int(declared) > max_bytes: + raise RuntimeError( + f"download exceeds the {max_bytes}-byte safety limit" + ) deadline = time.monotonic() + _NETWORK_TIMEOUT_SECONDS total = 0 - with open(dest, "wb") as out: - while True: - if time.monotonic() >= deadline: - raise TimeoutError( - f"download hop timed out: {current_url}" - ) - chunk = response.read(65536) - if not chunk: - return - total += len(chunk) - if max_bytes and total > max_bytes: - raise RuntimeError( - f"download exceeds the {max_bytes}-byte safety limit" - ) - out.write(chunk) + try: + with open(dest, "wb") as out: + while True: + if time.monotonic() >= deadline: + raise TimeoutError( + f"download hop timed out: {current_url}" + ) + chunk = response.read(65536) + if not chunk: + return + total += len(chunk) + if max_bytes and total > max_bytes: + raise RuntimeError( + f"download exceeds the {max_bytes}-byte " + f"safety limit" + ) + out.write(chunk) + except BaseException: + # Never leave a partial download behind for a later step to + # mistake for the complete asset. + _remove_quietly(dest) + raise raise RuntimeError("too many redirects") +def _remove_quietly(path) -> None: + try: + os.unlink(path) + except OSError: + pass + + def _verify_candidate(path: Path) -> None: """Require a staged native candidate to execute successfully.""" try: @@ -196,33 +236,157 @@ def _validate_archive_names(names, archive_names, casefold: bool = False): ) +def _validate_archive_limits(limits) -> None: + if ( + limits.compressed_bytes <= 0 + or limits.members <= 0 + or limits.member_bytes <= 0 + or limits.expanded_bytes <= 0 + ): + sys.exit("codebase-memory-mcp: invalid archive resource safety limits") + + +def _require_compressed_archive_within_limit(archive_path: str, limits) -> None: + status = os.stat(archive_path) + if not stat.S_ISREG(status.st_mode): + sys.exit("codebase-memory-mcp: release archive is not a regular file") + if status.st_size > limits.compressed_bytes: + sys.exit( + f"codebase-memory-mcp: release archive exceeds the " + f"{limits.compressed_bytes}-byte compressed safety limit" + ) + + +def _require_member_count_within_limit(count: int, limits) -> None: + if count > limits.members: + sys.exit( + f"codebase-memory-mcp: archive exceeds the {limits.members}-member " + f"safety limit" + ) + + +def _account_declared_archive_member( + name: str, size: int, declared_expanded: int, limits +) -> int: + """Check one member's declared size; return the new declared total.""" + if size < 0: + sys.exit( + f"codebase-memory-mcp: archive member has a negative declared " + f"size: {name!r}" + ) + if size > limits.member_bytes: + sys.exit( + f"codebase-memory-mcp: archive member {name!r} exceeds the " + f"{limits.member_bytes}-byte expanded safety limit" + ) + if declared_expanded > limits.expanded_bytes - size: + sys.exit( + f"codebase-memory-mcp: archive exceeds the {limits.expanded_bytes}-byte " + f"aggregate expanded safety limit" + ) + return declared_expanded + size + + +def _copy_archive_member_within_limits( + source, output, name: str, declared_size: int, actual_expanded: int, limits +) -> int: + """Copy a member while counting bytes; return the new actual total. + + Reads stop one byte past the smaller of the member and remaining aggregate + limits, so a member that yields more than it declares can never fill the + disk. The caller removes the target on failure. + """ + remaining = limits.expanded_bytes - actual_expanded + copy_limit = min(limits.member_bytes, remaining) + copied = 0 + while True: + wanted = min(_ARCHIVE_COPY_CHUNK_BYTES, copy_limit + 1 - copied) + if wanted <= 0: + break + chunk = source.read(wanted) + if not chunk: + break + copied += len(chunk) + if copied > copy_limit: + break + output.write(chunk) + if copied > limits.member_bytes: + sys.exit( + f"codebase-memory-mcp: archive member {name!r} exceeds the " + f"{limits.member_bytes}-byte actual expanded safety limit" + ) + if copied > remaining: + sys.exit( + f"codebase-memory-mcp: archive exceeds the {limits.expanded_bytes}-byte " + f"aggregate actual expanded safety limit" + ) + if copied != declared_size: + sys.exit( + f"codebase-memory-mcp: archive member {name!r} actual size {copied} " + f"does not match declared size {declared_size}" + ) + return actual_expanded + copied + + +def _write_archive_member_within_limits( + source, target: str, name: str, declared_size: int, actual_expanded: int, limits +) -> int: + try: + with open(target, "xb") as output: + return _copy_archive_member_within_limits( + source, output, name, declared_size, actual_expanded, limits + ) + except BaseException: + _remove_quietly(target) + raise + + def _safe_extract_tar( - tf, dest: str, archive_names=(), extract_names=(), ui: bool = False + tf, + dest: str, + archive_names=(), + extract_names=(), + ui: bool = False, + limits=_DEFAULT_ARCHIVE_LIMITS, ): """Validate a tar namespace, then extract only its authenticated runtime set.""" + _validate_archive_limits(limits) members = tf.getmembers() + _require_member_count_within_limit(len(members), limits) + declared_expanded = 0 for member in members: if not member.isfile(): sys.exit( f"codebase-memory-mcp: refusing unsafe tar entry " f"(not a regular root file: {member.name!r})" ) + declared_expanded = _account_declared_archive_member( + member.name, member.size, declared_expanded, limits + ) _validate_archive_names([member.name for member in members], archive_names) runtime_names = list(extract_names) by_name = {member.name: member for member in members} dest_abs = os.path.abspath(dest) + actual_expanded = 0 for name in runtime_names: source = tf.extractfile(by_name[name]) if source is None: sys.exit(f"codebase-memory-mcp: could not read archive member: {name}") target = os.path.join(dest_abs, name) - with source, open(target, "xb") as output: - shutil.copyfileobj(source, output) + with source: + actual_expanded = _write_archive_member_within_limits( + source, target, name, by_name[name].size, actual_expanded, limits + ) return tuple(runtime_names) def _safe_extract_zip( - zf, dest: str, archive_names=(), extract_names=(), ui: bool = False + zf, + dest: str, + archive_names=(), + extract_names=(), + ui: bool = False, + limits=_DEFAULT_ARCHIVE_LIMITS, ): """Extract a zipfile after validating its Windows-style namespace. @@ -230,9 +394,16 @@ def _safe_extract_zip( members before extraction prevents archive order from selecting the binary that a portable package-manager shim eventually executes. """ + _validate_archive_limits(limits) dest_abs = os.path.abspath(dest) names = [] - for info in zf.infolist(): + infos = zf.infolist() + _require_member_count_within_limit(len(infos), limits) + declared_expanded = 0 + for info in infos: + declared_expanded = _account_declared_archive_member( + info.filename, info.file_size, declared_expanded, limits + ) raw_name = info.filename name = raw_name.replace("\\", "/") segments_name = name[:-1] if name.endswith("/") else name @@ -266,10 +437,18 @@ def _safe_extract_zip( # Extract only the validated runtime files. This avoids relying on # platform-specific zip path rewriting and always creates regular files. runtime_names = list(extract_names) + actual_expanded = 0 for name in runtime_names: target = os.path.join(dest_abs, name) - with zf.open(name) as source, open(target, "xb") as output: - shutil.copyfileobj(source, output) + with zf.open(name) as source: + actual_expanded = _write_archive_member_within_limits( + source, + target, + name, + zf.getinfo(name).file_size, + actual_expanded, + limits, + ) return tuple(runtime_names) @@ -1179,7 +1358,7 @@ def _download(version: str) -> Path: with tempfile.TemporaryDirectory() as tmp: tmp_archive = os.path.join(tmp, f"cbm.{ext}") try: - _download_https(url, tmp_archive) + _download_https(url, tmp_archive, _MAX_RELEASE_ARCHIVE_BYTES) except (OSError, RuntimeError, urllib.error.URLError) as e: sys.exit( f"codebase-memory-mcp: download failed ({e})\n" @@ -1188,6 +1367,9 @@ def _download(version: str) -> Path: ) _verify_checksum(tmp_archive, archive, version) + _require_compressed_archive_within_limit( + tmp_archive, _DEFAULT_ARCHIVE_LIMITS + ) bin_name = ( _WINDOWS_BINARY_NAME if os_name == "windows" else "codebase-memory-mcp" diff --git a/pkg/pypi/tests/test_cli.py b/pkg/pypi/tests/test_cli.py index ca66fbdba..b6d62f7d2 100644 --- a/pkg/pypi/tests/test_cli.py +++ b/pkg/pypi/tests/test_cli.py @@ -1,5 +1,6 @@ import errno import hashlib +import io import json import os import stat @@ -835,3 +836,296 @@ _cli._publish_runtime_set( False, ) + +class _FakeHttpsResponse: + """Minimal stand-in for the opener's response: headers, geturl, read.""" + + def __init__(self, body: bytes, headers=None): + self._body = body + self._offset = 0 + self.headers = dict(headers or {}) + + def __enter__(self): + return self + + def __exit__(self, *exc_info): + return False + + def geturl(self): + return "https://example.invalid/release.tar.gz" + + def read(self, size=-1): + if size is None or size < 0: + size = len(self._body) - self._offset + chunk = self._body[self._offset : self._offset + size] + self._offset += len(chunk) + return chunk + + +def _limits_with(**overrides): + return _cli._DEFAULT_ARCHIVE_LIMITS._replace(**overrides) + + +def _write_tar_gz(archive, entries): + with tarfile.open(archive, "w:gz") as tf: + for name, data in entries: + info = tarfile.TarInfo(name) + info.size = len(data) + tf.addfile(info, io.BytesIO(data)) + + +def _write_zip(archive, entries): + with zipfile.ZipFile(archive, "w") as zf: + for name, data in entries: + zf.writestr(name, data) + + +def _release_entries(names): + return [(name, f"payload of {name}".encode()) for name in names] + + +class ArchiveLimitTests(unittest.TestCase): + URL = "https://example.invalid/release.tar.gz" + + def _download(self, response, destination, max_bytes): + with mock.patch.object(_cli._HTTPS_OPENER, "open", return_value=response): + _cli._download_https(self.URL, destination, max_bytes) + + def test_download_rejects_a_declared_length_over_the_limit(self): + with tempfile.TemporaryDirectory() as root: + destination = os.path.join(root, "release.tar.gz") + response = _FakeHttpsResponse(b"", {"Content-Length": "9"}) + with self.assertRaises(RuntimeError) as raised: + self._download(response, destination, 8) + self.assertIn("8-byte safety limit", str(raised.exception)) + self.assertFalse(os.path.exists(destination)) + + def test_download_rejects_a_body_over_the_limit_and_removes_the_partial_file( + self, + ): + with tempfile.TemporaryDirectory() as root: + destination = os.path.join(root, "release.tar.gz") + response = _FakeHttpsResponse(b"x" * 1024) + with self.assertRaises(RuntimeError) as raised: + self._download(response, destination, 8) + self.assertIn("8-byte safety limit", str(raised.exception)) + self.assertFalse(os.path.exists(destination)) + + def test_download_within_the_limit_stores_the_body(self): + with tempfile.TemporaryDirectory() as root: + destination = os.path.join(root, "release.tar.gz") + response = _FakeHttpsResponse(b"payload", {"Content-Length": "7"}) + self._download(response, destination, 1024) + with open(destination, "rb") as stored: + self.assertEqual(stored.read(), b"payload") + + def test_download_counts_bytes_when_the_declared_length_is_not_a_number(self): + # A superscript two passes str.isdigit but not int(); such values and + # plain junk must fall through to the byte counter, never crash. + for declared in ("²", "abc", "", "1e3", "-1"): + with self.subTest(declared=declared), tempfile.TemporaryDirectory() as root: + destination = os.path.join(root, "release.tar.gz") + response = _FakeHttpsResponse(b"x" * 1024, {"Content-Length": declared}) + with self.assertRaises(RuntimeError) as raised: + self._download(response, destination, 8) + self.assertIn("8-byte safety limit", str(raised.exception)) + self.assertFalse(os.path.exists(destination)) + with tempfile.TemporaryDirectory() as root: + destination = os.path.join(root, "release.tar.gz") + response = _FakeHttpsResponse(b"payload", {"Content-Length": "²"}) + self._download(response, destination, 1024) + with open(destination, "rb") as stored: + self.assertEqual(stored.read(), b"payload") + + def test_unix_tar_rejects_more_members_than_the_limit(self): + with tempfile.TemporaryDirectory() as root: + archive = Path(root) / "members.tar.gz" + _write_tar_gz(archive, [("one", b"a"), ("two", b"b"), ("three", b"c")]) + destination = Path(root) / "extract" + destination.mkdir() + with tarfile.open(archive) as tf, self.assertRaises(SystemExit) as raised: + _cli._safe_extract_tar( + tf, + str(destination), + ("one", "two", "three"), + ("one",), + limits=_limits_with(members=2), + ) + self.assertIn("2-member safety limit", str(raised.exception)) + self.assertFalse((destination / "one").exists()) + + def test_unix_tar_rejects_a_member_declared_over_the_limit(self): + with tempfile.TemporaryDirectory() as root: + archive = Path(root) / "member.tar.gz" + _write_tar_gz(archive, [("codebase-memory-mcp", b"x" * 65)]) + destination = Path(root) / "extract" + destination.mkdir() + with tarfile.open(archive) as tf, self.assertRaises(SystemExit) as raised: + _cli._safe_extract_tar( + tf, + str(destination), + ("codebase-memory-mcp",), + ("codebase-memory-mcp",), + limits=_limits_with(member_bytes=64), + ) + self.assertIn( + "'codebase-memory-mcp' exceeds the 64-byte expanded safety limit", + str(raised.exception), + ) + self.assertFalse((destination / "codebase-memory-mcp").exists()) + + def test_unix_tar_rejects_a_declared_aggregate_over_the_limit(self): + with tempfile.TemporaryDirectory() as root: + archive = Path(root) / "aggregate.tar.gz" + _write_tar_gz(archive, [("one", b"a" * 40), ("two", b"b" * 40)]) + destination = Path(root) / "extract" + destination.mkdir() + with tarfile.open(archive) as tf, self.assertRaises(SystemExit) as raised: + _cli._safe_extract_tar( + tf, + str(destination), + ("one", "two"), + ("one",), + limits=_limits_with(member_bytes=64, expanded_bytes=64), + ) + self.assertIn( + "64-byte aggregate expanded safety limit", str(raised.exception) + ) + + def test_unix_tar_within_the_limits_extracts_only_the_executable(self): + with tempfile.TemporaryDirectory() as root: + archive = Path(root) / "release.tar.gz" + _write_tar_gz(archive, _release_entries(_cli._UNIX_ARCHIVE_NAMES)) + destination = Path(root) / "extract" + destination.mkdir() + with tarfile.open(archive) as tf: + extracted = _cli._safe_extract_tar( + tf, + str(destination), + _cli._UNIX_ARCHIVE_NAMES, + ("codebase-memory-mcp",), + ) + self.assertEqual(extracted, ("codebase-memory-mcp",)) + self.assertEqual( + (destination / "codebase-memory-mcp").read_bytes(), + b"payload of codebase-memory-mcp", + ) + self.assertEqual(os.listdir(destination), ["codebase-memory-mcp"]) + + def test_archive_copy_stops_a_member_that_yields_more_than_the_limit(self): + source = io.BytesIO(b"x" * 100) + output = io.BytesIO() + with self.assertRaises(SystemExit) as raised: + _cli._copy_archive_member_within_limits( + source, + output, + "member", + 100, + 0, + _limits_with(member_bytes=64, expanded_bytes=1024), + ) + self.assertIn( + "'member' exceeds the 64-byte actual expanded safety limit", + str(raised.exception), + ) + # Reading stopped one byte past the limit; nothing beyond it was kept. + self.assertEqual(source.tell(), 65) + self.assertLessEqual(len(output.getvalue()), 64) + + def test_archive_copy_rejects_a_member_that_yields_more_than_it_declares(self): + source = io.BytesIO(b"x" * 100) + with self.assertRaises(SystemExit) as raised: + _cli._copy_archive_member_within_limits( + source, + io.BytesIO(), + "member", + 10, + 0, + _limits_with(member_bytes=1024, expanded_bytes=1024), + ) + self.assertIn( + "'member' actual size 100 does not match declared size 10", + str(raised.exception), + ) + + def test_archive_copy_rejects_an_aggregate_over_the_actual_limit(self): + with self.assertRaises(SystemExit) as raised: + _cli._copy_archive_member_within_limits( + io.BytesIO(b"abcd"), + io.BytesIO(), + "member", + 4, + 3, + _limits_with(member_bytes=10, expanded_bytes=5), + ) + self.assertIn( + "5-byte aggregate actual expanded safety limit", str(raised.exception) + ) + + def test_archive_member_write_removes_the_partial_file_on_failure(self): + with tempfile.TemporaryDirectory() as root: + target = os.path.join(root, "codebase-memory-mcp") + with self.assertRaises(SystemExit): + _cli._write_archive_member_within_limits( + io.BytesIO(b"x" * 100), + target, + "codebase-memory-mcp", + 10, + 0, + _limits_with(member_bytes=1024, expanded_bytes=1024), + ) + self.assertFalse(os.path.exists(target)) + + def test_windows_zip_rejects_more_members_than_the_limit(self): + with tempfile.TemporaryDirectory() as root: + archive = Path(root) / "members.zip" + _write_zip(archive, [("one", b"a"), ("two", b"b"), ("three", b"c")]) + destination = Path(root) / "extract" + destination.mkdir() + with zipfile.ZipFile(archive) as zf, self.assertRaises(SystemExit) as raised: + _cli._safe_extract_zip( + zf, + str(destination), + ("one", "two", "three"), + ("one",), + limits=_limits_with(members=2), + ) + self.assertIn("2-member safety limit", str(raised.exception)) + + def test_windows_zip_rejects_a_member_declared_over_the_limit(self): + with tempfile.TemporaryDirectory() as root: + archive = Path(root) / "member.zip" + _write_zip(archive, [(_cli._WINDOWS_BINARY_NAME, b"x" * 65)]) + destination = Path(root) / "extract" + destination.mkdir() + with zipfile.ZipFile(archive) as zf, self.assertRaises(SystemExit) as raised: + _cli._safe_extract_zip( + zf, + str(destination), + (_cli._WINDOWS_BINARY_NAME,), + (_cli._WINDOWS_BINARY_NAME,), + limits=_limits_with(member_bytes=64), + ) + self.assertIn("64-byte expanded safety limit", str(raised.exception)) + self.assertFalse((destination / _cli._WINDOWS_BINARY_NAME).exists()) + + def test_windows_zip_within_the_limits_extracts_only_the_executable(self): + with tempfile.TemporaryDirectory() as root: + archive = Path(root) / "release.zip" + _write_zip(archive, _release_entries(_cli._WINDOWS_ARCHIVE_NAMES)) + destination = Path(root) / "extract" + destination.mkdir() + with zipfile.ZipFile(archive) as zf: + extracted = _cli._safe_extract_zip( + zf, + str(destination), + _cli._WINDOWS_ARCHIVE_NAMES, + (_cli._WINDOWS_BINARY_NAME,), + ) + self.assertEqual(extracted, (_cli._WINDOWS_BINARY_NAME,)) + self.assertEqual( + (destination / _cli._WINDOWS_BINARY_NAME).read_bytes(), + b"payload of codebase-memory-mcp.exe", + ) + self.assertEqual(os.listdir(destination), [_cli._WINDOWS_BINARY_NAME]) +