From 4bb01598771db88f89c2d0d4c124662457861c7c Mon Sep 17 00:00:00 2001 From: Oto Macenauer Date: Fri, 14 Aug 2026 12:34:21 +0200 Subject: [PATCH] fix: harden artifact fetching and extraction in the build pipeline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three related weaknesses in how sub-app artifacts reach apps/, all in the path that consumes other repositories' release tarballs. The GitHub API helper built a curl command as a single shell string with GITHUB_TOKEN inlined, so the token became process argv (readable locally) and was echoed back in the error execSync throws on a failed request. The same string interpolated apps.json values with no escaping. Both API calls and asset downloads now use fetch with the token as a request header, and registry repo/version values are validated before use. Archives were extracted with no validation, so a compromised doc repo could write outside the staging directory or smuggle in a symlink that the later HTML crawl would follow. Extraction now rejects absolute, drive-letter and ".."-escaping members before writing anything, and refuses declared link members — read from the archive listing rather than from the extracted result, since Windows silently drops symlink members it lacks the privilege to create. fetch-apps.js also shelled out to cp -r and to tar with absolute paths, neither of which works on Windows, while build-vite.js already carried a documented workaround for exactly that. Both now share scripts/artifacts.js. Closes #43 Closes #44 Closes #56 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01QqFK6yffibtCBTF8xZ4hXW --- CLAUDE.md | 1 + scripts/artifacts.js | 155 ++++++++++++++++++++++++++++++++ scripts/build-vite.js | 48 +++------- scripts/fetch-apps.js | 114 ++++++++++++++--------- tests/artifact-safety.spec.js | 165 ++++++++++++++++++++++++++++++++++ 5 files changed, 401 insertions(+), 82 deletions(-) create mode 100644 scripts/artifacts.js create mode 100644 tests/artifact-safety.spec.js diff --git a/CLAUDE.md b/CLAUDE.md index 3d7fd98..0617256 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -75,6 +75,7 @@ Orchestrator: `scripts/build-vite.js`. Flags: `--local`, `--headless`, `--path-p - `src/utils/single-page.js` — Bundle manifest reading/validation + registry expansion, shared by both fetch paths and by Astro - `scripts/build-vite.js` — Build orchestrator (3-step pipeline) - `scripts/fetch-apps.js` — GitHub Release artifact downloader +- `scripts/artifacts.js` — Safe tarball extraction + tree copy, shared by both fetch paths. Validates archive members (no traversal, no absolute paths, no symlinks) before anything is written, and replaces the old `cp -r`/`tar` shell-outs so the build runs on Windows - `actions/publish-single-page-docs/` — Reusable GitHub Action that turns a repo's markdown into a single-page bundle ### Three Onboarding Types diff --git a/scripts/artifacts.js b/scripts/artifacts.js new file mode 100644 index 0000000..18d28d1 --- /dev/null +++ b/scripts/artifacts.js @@ -0,0 +1,155 @@ +/** + * artifacts.js — safe extraction and copying of sub-app artifacts. + * + * Every artifact this build consumes is a `dist.tar.gz` produced by *another* + * repository's release pipeline, so its contents are untrusted input: a + * compromised (or merely careless) doc repo must not be able to write outside + * the staging directory, and must not be able to smuggle a symlink into the + * published site. + * + * Both onboarding paths share this module — scripts/fetch-apps.js (GitHub + * releases) and scripts/build-vite.js (`prebuilt`/`localPath`) — so the two + * cannot drift on the safety checks the way they previously drifted on + * portability. + */ + +import { execFileSync } from 'node:child_process'; +import { copyFileSync, existsSync, lstatSync, mkdirSync, readdirSync, rmSync } from 'node:fs'; +import { basename, dirname, join } from 'node:path'; + +/** Forward slashes — GNU tar in git-bash rejects backslash-separated -C targets. */ +const posix = (p) => p.replace(/\\/g, '/'); + +/** + * Rejects an archive member that would escape the extraction root. + * + * tar itself strips a leading `/` and GNU tar refuses `..` members, but that is + * a defence we neither control nor can rely on across GNU tar and the bsdtar + * shipped in Windows System32. Checking the listing first makes the guarantee + * ours and produces an error naming the offending entry. + */ +function assertNoTraversal(entries, label) { + for (const entry of entries) { + const name = entry.replace(/\\/g, '/'); + if (name.startsWith('/')) { + throw new Error(`${label}: archive member "${entry}" is an absolute path — refusing to extract.`); + } + if (/^[a-z]:/i.test(name)) { + throw new Error(`${label}: archive member "${entry}" carries a drive letter — refusing to extract.`); + } + if (name.split('/').includes('..')) { + throw new Error(`${label}: archive member "${entry}" escapes the archive root via ".." — refusing to extract.`); + } + } +} + +/** + * Rejects link members declared in the archive. + * + * A symlink that survived into apps/ would be followed later by the HTML crawl + * and the asset copy, which is how a doc artifact could publish a file from + * outside its own tree — a CI secret, say. + * + * This reads the *declaration* rather than the extracted result because the two + * differ by platform: Windows silently drops symlink members when the process + * lacks the create-symlink privilege, so a post-extraction check alone would + * pass on a developer machine and only fail on Linux CI. GNU tar and bsdtar + * disagree about the rest of the verbose line, but both start it with the mode + * string, whose first character is `l` for a symlink and `h` for a hardlink. + */ +function assertNoLinkMembers(verboseListing, label) { + for (const line of verboseListing.split('\n')) { + const trimmed = line.trim(); + if (!trimmed) continue; + const type = trimmed[0]; + if (type === 'l' || type === 'h') { + throw new Error( + `${label}: archive declares a ${type === 'l' ? 'symlink' : 'hardlink'} member ` + + `(${trimmed}) — refusing to use it. Doc artifacts must contain regular files only.`, + ); + } + } +} + +/** + * Rejects symlinks anywhere under `dir`, after extraction. + * + * Belt and braces with assertNoLinkMembers: `lstat` behaves the same everywhere + * and catches anything the listing parse missed. + */ +function assertNoSymlinks(dir, label, root = dir) { + for (const entry of readdirSync(dir, { withFileTypes: true })) { + const full = join(dir, entry.name); + if (entry.isSymbolicLink()) { + throw new Error( + `${label}: archive contains a symlink at "${posix(full.slice(root.length + 1))}" — ` + + `refusing to use it. Doc artifacts must contain regular files only.`, + ); + } + if (entry.isDirectory()) assertNoSymlinks(full, label, root); + } +} + +/** + * Extracts a .tar.gz into `destDir`, validating it first. + * + * Portability: GNU tar (git-bash) parses a leading "C:" in the ARCHIVE name as a + * remote host, so the command runs from the tarball's own directory and passes + * only its basename to -f. The -C target is not parsed that way, it just needs + * forward slashes. This avoids --force-local, which bsdtar rejects. + * + * @param {string} tarPath - path to the .tar.gz + * @param {string} destDir - directory to extract into (created if absent) + * @param {string} label - human-readable origin, used in error messages + */ +export function extractTarball(tarPath, destDir, label) { + mkdirSync(destDir, { recursive: true }); + + const cwd = dirname(tarPath); + const name = basename(tarPath); + + const opts = { cwd, encoding: 'utf8', stdio: ['pipe', 'pipe', 'pipe'] }; + const entries = execFileSync('tar', ['-tzf', name], opts) + .split('\n').map((l) => l.trim()).filter(Boolean); + if (entries.length === 0) throw new Error(`${label}: archive is empty.`); + assertNoTraversal(entries, label); + assertNoLinkMembers(execFileSync('tar', ['-tvzf', name], opts), label); + + execFileSync('tar', ['-xzf', name, '-C', posix(destDir)], { cwd, stdio: 'pipe' }); + assertNoSymlinks(destDir, label); +} + +/** + * Materialises a local artifact into a staging directory. + * + * The artifact may be a `.tar.gz` tarball (as published to GitHub Releases) or a + * directory. Tarballs are extracted under `stageRoot/{name}`; directories are + * used in place. + */ +export function stageArtifact(srcPath, name, stageRoot, label = name) { + if (!lstatSync(srcPath).isFile()) return srcPath; + + const stageDir = join(stageRoot, name); + if (existsSync(stageDir)) rmSync(stageDir, { recursive: true }); + extractTarball(srcPath, stageDir, label); + return stageDir; +} + +/** + * Recursively copies a directory tree, skipping symlinks. + * + * Replaces the `cp -r` this build used to shell out to, which does not exist on + * Windows outside a POSIX shell. `readdirSync(withFileTypes)` also avoids a + * `statSync` syscall per entry, and reports symlinks without following them. + */ +export function copyDir(src, dest) { + mkdirSync(dest, { recursive: true }); + const entries = readdirSync(src, { withFileTypes: true }).sort((a, b) => (a.name < b.name ? -1 : 1)); + for (const entry of entries) { + if (entry.isSymbolicLink()) continue; + const srcPath = join(src, entry.name); + const destPath = join(dest, entry.name); + if (entry.isDirectory()) copyDir(srcPath, destPath); + else if (entry.isFile()) copyFileSync(srcPath, destPath); + } +} diff --git a/scripts/build-vite.js b/scripts/build-vite.js index fd5b9dc..f4b6bc9 100644 --- a/scripts/build-vite.js +++ b/scripts/build-vite.js @@ -14,11 +14,12 @@ * Sub-app pages are rendered by src/pages/[...path].astro via getStaticPaths. */ -import { existsSync, readFileSync, writeFileSync, mkdirSync, rmSync, copyFileSync, readdirSync, statSync } from 'fs'; -import { join, dirname, basename, resolve, isAbsolute } from 'path'; -import { fileURLToPath } from 'url'; -import { homedir } from 'os'; -import { execSync } from 'child_process'; +import { existsSync, readFileSync, writeFileSync, mkdirSync, rmSync, copyFileSync, readdirSync, statSync } from 'node:fs'; +import { join, dirname, resolve, isAbsolute } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { homedir } from 'node:os'; +import { execSync } from 'node:child_process'; +import { copyDir, stageArtifact } from './artifacts.js'; import { fetchApps } from './fetch-apps.js'; import { BUNDLE_MANIFEST, bundleDirName, bundleKey, expandBundle, findBundleRoot, @@ -39,38 +40,8 @@ const ok = (msg) => console.log('\x1b[32m✓\x1b[0m ' + msg); const warn = (msg) => console.warn('\x1b[33m⚠\x1b[0m ' + msg); const step = (msg) => console.log('\n\x1b[1m' + msg + '\x1b[0m'); -function copyDir(src, dest) { - mkdirSync(dest, { recursive: true }); - for (const entry of readdirSync(src).sort()) { - const srcPath = join(src, entry); - const destPath = join(dest, entry); - if (statSync(srcPath).isDirectory()) copyDir(srcPath, destPath); - else copyFileSync(srcPath, destPath); - } -} - -/** - * Materialises a local artifact into a staging directory. - * - * The artifact may be a `.tar.gz` tarball (as published to GitHub Releases) or a - * directory. Tarballs are extracted under tmp/prebuilt/{name}/; directories are - * used in place. - */ -function stageArtifact(srcPath, name) { - if (!statSync(srcPath).isFile()) return srcPath; - - const stageDir = join(ROOT, 'tmp', 'prebuilt', name); - if (existsSync(stageDir)) rmSync(stageDir, { recursive: true }); - mkdirSync(stageDir, { recursive: true }); - // Portable across GNU tar (git-bash) and bsdtar (Windows System32): GNU tar - // parses a leading "C:" in the ARCHIVE name as a remote host, so run from the - // tarball's dir and pass only its basename to -f (the -C target is not parsed - // that way). This avoids --force-local, which bsdtar rejects. - const posix = (p) => p.replace(/\\/g, '/'); - execSync('tar -xzf "' + basename(srcPath) + '" -C "' + posix(stageDir) + '"', - { cwd: dirname(srcPath), stdio: 'pipe' }); - return stageDir; -} +/** Where `prebuilt` tarballs are unpacked before being copied into apps/. */ +const STAGE_ROOT = join(ROOT, 'tmp', 'prebuilt'); /** Expands a registry path (`~` and repo-relative forms allowed) to an absolute one. */ function artifactPath(raw) { @@ -98,8 +69,9 @@ function resolveArtifactPath(app, raw, label) { * @returns {Array|null} expanded app entries for a single-page bundle, else null */ function preparePrebuilt(app) { + const label = app.slug ?? bundleKey(app); const srcPath = resolveArtifactPath(app, app.prebuilt, 'prebuilt'); - const stageDir = stageArtifact(srcPath, bundleDirName(app.slug ?? bundleKey(app))); + const stageDir = stageArtifact(srcPath, bundleDirName(label), STAGE_ROOT, label); if (isSinglePage(app)) return installBundle(app, stageDir, 'prebuilt'); diff --git a/scripts/fetch-apps.js b/scripts/fetch-apps.js index a212fd4..654cfbe 100644 --- a/scripts/fetch-apps.js +++ b/scripts/fetch-apps.js @@ -10,10 +10,13 @@ * Legacy: also keeps tmp/apps/{slug}/ populated for non-Vite paths. */ -import { execSync, spawnSync } from 'child_process'; -import { mkdirSync, rmSync, existsSync, writeFileSync, readFileSync } from 'fs'; -import { join, dirname } from 'path'; -import { fileURLToPath } from 'url'; +import { spawnSync } from 'node:child_process'; +import { createWriteStream, mkdirSync, rmSync, existsSync, copyFileSync, readFileSync } from 'node:fs'; +import { join, dirname } from 'node:path'; +import { pipeline } from 'node:stream/promises'; +import { Readable } from 'node:stream'; +import { fileURLToPath } from 'node:url'; +import { copyDir, extractTarball } from './artifacts.js'; import { BUNDLE_MANIFEST, bundleDirName, bundleKey, expandBundle, findBundleRoot, isSinglePage, readBundleManifest, toRegistryEntry, @@ -36,20 +39,48 @@ function warn(msg) { process.stderr.write(` \x1b[33m⚠\x1b[0m ${msg}\n`); } function ok(msg) { process.stdout.write(` \x1b[32m✓\x1b[0m ${msg}\n`); } function fail(msg) { throw new Error(`\x1b[31m✗\x1b[0m ${msg}`); } +/** `owner/name`, the only shape a registry `repo` may take. */ +const REPO_RE = /^[A-Za-z0-9._-]+\/[A-Za-z0-9._-]+$/; +/** Git tag characters we are willing to put in a URL path. */ +const VERSION_RE = /^[A-Za-z0-9._/-]+$/; + +/** + * Rejects registry values that must never reach a URL or a subprocess argument. + * + * apps.json is a reviewed file, but it is also the one file an onboarding PR + * edits, so its values are validated rather than trusted. + */ +function assertSafeTarget(repo, version, label) { + if (typeof repo !== 'string' || !REPO_RE.test(repo)) { + fail(`${label}: "repo" must look like "owner/name", got ${JSON.stringify(repo)}.`); + } + if (typeof version !== 'string' || !VERSION_RE.test(version)) { + fail(`${label}: "version" ${JSON.stringify(version)} contains characters that are not valid in a git tag.`); + } +} + /** * Makes a GitHub API request. Uses GITHUB_TOKEN when available, otherwise * delegates to `gh api` CLI so developers don't need to set tokens locally. + * + * The token is passed as a request header, never as part of a command line: + * a shell string would put it in the process argv (readable by any local + * process, and echoed back in the error execSync throws on a failed request). */ -function ghApi(path) { +async function ghApi(path) { if (GITHUB_TOKEN) { - const res = execSync( - `curl -fsSL -H "Authorization: Bearer ${GITHUB_TOKEN}" -H "Accept: application/vnd.github+json" "${API_BASE}${path}"`, - { encoding: 'utf8', stdio: ['pipe', 'pipe', 'pipe'] } - ); - return JSON.parse(res); + const res = await fetch(`${API_BASE}${path}`, { + headers: { + Authorization: `Bearer ${GITHUB_TOKEN}`, + Accept: 'application/vnd.github+json', + 'X-GitHub-Api-Version': '2022-11-28', + }, + }); + if (!res.ok) fail(`GitHub API request failed: ${res.status} ${res.statusText} — ${path}`); + return res.json(); } - // Fallback: gh CLI + // Fallback: gh CLI. spawnSync takes an argv array, so nothing is shell-parsed. const result = spawnSync('gh', ['api', path], { encoding: 'utf8' }); if (result.status !== 0) fail(`gh api ${path} failed:\n${result.stderr}`); return JSON.parse(result.stdout); @@ -58,34 +89,28 @@ function ghApi(path) { /** * Downloads a GitHub Release asset to a local path. * - * Uses `gh release download` (preferred) because it correctly handles - * the GitHub → S3 redirect for private repo assets — curl with a Bearer - * token breaks on the S3 redirect since S3 rejects the auth header. - * - * Falls back to the GitHub API asset endpoint with --no-location + manual - * redirect handling if gh CLI is unavailable. + * Uses the API asset endpoint with the numeric asset ID rather than + * browser_download_url: for a private repo the latter redirects to S3, which + * rejects requests carrying an Authorization header. `fetch` follows the + * redirect and drops the header across origins, which is exactly what is wanted. */ -function downloadAsset(repo, releaseTag, assetId, assetName, destPath) { - // Download via GitHub API asset endpoint using the numeric asset ID. - // Must use the asset ID URL (not browser_download_url) to avoid the - // broken Bearer-token-on-S3-redirect issue with private repos. +async function downloadAsset(repo, assetId, assetName, destPath) { if (!GITHUB_TOKEN) { throw new Error( 'GITHUB_TOKEN is not set. Cannot download release assets from private repos.' ); } - const assetApiUrl = `${API_BASE}/repos/${repo}/releases/assets/${assetId}`; - const cmd = [ - 'curl', '-fsSL', - '-H', `Authorization: Bearer ${GITHUB_TOKEN}`, - '-H', 'Accept: application/octet-stream', - '-L', assetApiUrl, - '-o', destPath, - ]; - const result = spawnSync(cmd[0], cmd.slice(1), { stdio: 'inherit' }); - if (result.status !== 0) { - throw new Error(`curl download failed for ${repo} asset ${assetName}`); + const res = await fetch(`${API_BASE}/repos/${repo}/releases/assets/${assetId}`, { + headers: { + Authorization: `Bearer ${GITHUB_TOKEN}`, + Accept: 'application/octet-stream', + 'X-GitHub-Api-Version': '2022-11-28', + }, + }); + if (!res.ok || !res.body) { + throw new Error(`Download failed for ${repo} asset ${assetName}: ${res.status} ${res.statusText}`); } + await pipeline(Readable.fromWeb(res.body), createWriteStream(destPath)); } /** @@ -93,10 +118,10 @@ function downloadAsset(repo, releaseTag, assetId, assetName, destPath) { * - version "latest": fetches the latest release * - version "v1.2.3": fetches that specific tag */ -function fetchRelease(repo, version) { +async function fetchRelease(repo, version) { const path = version === 'latest' ? `/repos/${repo}/releases/latest` - : `/repos/${repo}/releases/tags/${version}`; + : `/repos/${repo}/releases/tags/${encodeURIComponent(version)}`; return ghApi(path); } @@ -130,6 +155,7 @@ function findDistAsset(release, repo) { async function fetchBundle(app) { const { repo, version = 'latest' } = app; const key = bundleKey(app); + assertSafeTarget(repo, version, key); console.log(`\n\x1b[1m[${key}]\x1b[0m Fetching single-page bundle from ${repo}@${version}`); @@ -140,7 +166,7 @@ async function fetchBundle(app) { log(`Fetching release info (${version})…`); let release; try { - release = fetchRelease(repo, version); + release = await fetchRelease(repo, version); } catch (err) { fail(`Could not fetch release for ${repo}@${version}: ${err.message}`); } @@ -149,10 +175,10 @@ async function fetchBundle(app) { const asset = findDistAsset(release, repo); const tarPath = join(workDir, 'dist.tar.gz'); log(`Downloading ${asset.name} (${(asset.size / 1024).toFixed(1)} KB)…`); - downloadAsset(repo, release.tag_name, asset.id, asset.name, tarPath); + await downloadAsset(repo, asset.id, asset.name, tarPath); log('Extracting…'); - execSync(`tar -xzf "${tarPath}" -C "${workDir}"`, { stdio: 'pipe' }); + extractTarball(tarPath, workDir, `${key}@${release.tag_name}`); const bundleRoot = findBundleRoot(workDir); if (!bundleRoot) { @@ -168,7 +194,7 @@ async function fetchBundle(app) { for (const doc of docs) { const appsSlugDir = join(APPS_DIR, doc.slug); if (existsSync(appsSlugDir)) rmSync(appsSlugDir, { recursive: true }); - execSync(`cp -r "${doc.docDir}" "${appsSlugDir}"`, { stdio: 'pipe' }); + copyDir(doc.docDir, appsSlugDir); const html = readFileSync(join(appsSlugDir, doc.entryPoint), 'utf8'); if (!html.includes('data-mp-headless="true"')) { @@ -209,6 +235,7 @@ export async function fetchApps(apps) { } const { repo, slug, version = 'latest' } = app; + assertSafeTarget(repo, version, slug); console.log(`\n\x1b[1m[${slug}]\x1b[0m Fetching from ${repo}@${version}`); @@ -222,7 +249,7 @@ export async function fetchApps(apps) { log(`Fetching release info (${version})…`); let release; try { - release = fetchRelease(repo, version); + release = await fetchRelease(repo, version); } catch (err) { fail(`Could not fetch release for ${repo}@${version}: ${err.message}`); } @@ -234,11 +261,11 @@ export async function fetchApps(apps) { // 3. Download artifact log(`Downloading ${asset.name} (${(asset.size / 1024).toFixed(1)} KB)…`); - downloadAsset(repo, release.tag_name, asset.id, asset.name, tarPath); + await downloadAsset(repo, asset.id, asset.name, tarPath); - // 4. Extract + // 4. Extract — validated against traversal and symlink members first. log('Extracting…'); - execSync(`tar -xzf "${tarPath}" -C "${appDir}"`, { stdio: 'pipe' }); + extractTarball(tarPath, appDir, `${slug}@${release.tag_name}`); // Determine extracted dist location (may be dist/ or root-level files) const distDir = existsSync(join(appDir, 'dist')) @@ -248,13 +275,12 @@ export async function fetchApps(apps) { // 4b. Mirror into apps/{slug}/ for the Vite dev server and build pipeline const appsSlugDir = join(APPS_DIR, slug); if (existsSync(appsSlugDir)) rmSync(appsSlugDir, { recursive: true }); - execSync(`cp -r "${distDir}" "${appsSlugDir}"`, { stdio: 'pipe' }); + copyDir(distDir, appsSlugDir); // 5. Validate marketplace.json exists (in the tarball root, not dist/) const manifestInTar = join(appDir, 'marketplace.json'); // Also copy marketplace.json if present alongside the dist dir if (existsSync(manifestInTar)) { - const { copyFileSync } = await import('fs'); copyFileSync(manifestInTar, join(appsSlugDir, 'marketplace.json')); } log(`Synced to apps/${slug}/`); diff --git a/tests/artifact-safety.spec.js b/tests/artifact-safety.spec.js new file mode 100644 index 0000000..32d8755 --- /dev/null +++ b/tests/artifact-safety.spec.js @@ -0,0 +1,165 @@ +/** + * tests/artifact-safety.spec.js + * + * Unit tests for scripts/artifacts.js — the extraction guard that stands between + * an onboarding repo's release tarball and this repository's filesystem. + * + * Release artifacts come from other repositories, so they are untrusted input. A + * tarball that escapes its extraction root, or that smuggles in a symlink the + * later HTML crawl would follow, must be refused with a message naming the + * offending member — not extracted and dealt with afterwards. + * + * The fixtures are written byte-by-byte rather than produced with `tar`, because + * a traversal member is exactly what `tar` refuses to *create*, and because it + * keeps the test identical on GNU tar and the bsdtar shipped in Windows. + */ + +import { test, expect } from '@playwright/test'; +import { gzipSync } from 'node:zlib'; +import { mkdtempSync, mkdirSync, rmSync, writeFileSync, existsSync, readFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + +import { extractTarball, copyDir } from '../scripts/artifacts.js'; + +const BLOCK = 512; + +/** Writes a NUL-padded field into a header block. */ +function field(block, offset, length, value) { + Buffer.from(String(value)).copy(block, offset, 0, Math.min(length - 1, Buffer.byteLength(String(value)))); +} + +/** + * Builds one 512-byte ustar header. + * + * @param {string} name - member name as stored in the archive + * @param {number} size - content length in bytes + * @param {string} typeflag - '0' regular file, '2' symlink, '5' directory + * @param {string} linkname - target, for symlinks + */ +function header(name, size, typeflag = '0', linkname = '') { + const block = Buffer.alloc(BLOCK, 0); + field(block, 0, 100, name); + field(block, 100, 8, '0000644'); + field(block, 108, 8, '0000000'); + field(block, 116, 8, '0000000'); + field(block, 124, 12, size.toString(8).padStart(11, '0')); + field(block, 136, 12, '00000000000'); + // typeflag is exactly one byte with no NUL terminator — written directly + // rather than through field(), whose padding would consume the only slot. + block[156] = typeflag.charCodeAt(0); + field(block, 157, 100, linkname); + field(block, 257, 6, 'ustar'); + block.write('00', 263, 2, 'ascii'); + + // Checksum is computed with the checksum field itself read as eight spaces. + block.fill(0x20, 148, 156); + let sum = 0; + for (const byte of block) sum += byte; + field(block, 148, 8, sum.toString(8).padStart(6, '0')); + block[154] = 0; + block[155] = 0x20; + return block; +} + +/** Packs `entries` into a gzipped tar and writes it to `outPath`. */ +function writeTarball(outPath, entries) { + const blocks = []; + for (const entry of entries) { + const content = Buffer.from(entry.content ?? ''); + blocks.push(header(entry.name, entry.typeflag === '2' ? 0 : content.length, entry.typeflag ?? '0', entry.linkname ?? '')); + if (entry.typeflag !== '2' && content.length > 0) { + const padded = Buffer.alloc(Math.ceil(content.length / BLOCK) * BLOCK, 0); + content.copy(padded); + blocks.push(padded); + } + } + blocks.push(Buffer.alloc(BLOCK * 2, 0)); // end-of-archive marker + writeFileSync(outPath, gzipSync(Buffer.concat(blocks))); +} + +let workDir; + +test.beforeEach(() => { + workDir = mkdtempSync(join(tmpdir(), 'kb-artifact-safety-')); +}); + +test.afterEach(() => { + rmSync(workDir, { recursive: true, force: true }); +}); + +test.describe('extractTarball', () => { + test('extracts a well-formed bundle', () => { + const tar = join(workDir, 'dist.tar.gz'); + writeTarball(tar, [ + { name: 'bundle.json', content: '{"marketplaceVersion":"1"}' }, + { name: 'my-doc/index.html', content: '' }, + ]); + + const dest = join(workDir, 'out'); + extractTarball(tar, dest, 'good-bundle'); + + expect(readFileSync(join(dest, 'bundle.json'), 'utf8')).toContain('marketplaceVersion'); + expect(readFileSync(join(dest, 'my-doc', 'index.html'), 'utf8')).toContain('data-mp-headless'); + }); + + test('refuses a member that escapes the root via ".."', () => { + const tar = join(workDir, 'evil.tar.gz'); + writeTarball(tar, [ + { name: 'index.html', content: 'ok' }, + { name: '../../pwned.txt', content: 'escaped' }, + ]); + + const dest = join(workDir, 'out'); + expect(() => extractTarball(tar, dest, 'evil-repo')) + .toThrow(/evil-repo.*escapes the archive root/s); + + // Nothing may have been written outside — the guard runs before extraction. + expect(existsSync(join(workDir, 'pwned.txt'))).toBe(false); + expect(existsSync(join(dest, 'index.html'))).toBe(false); + }); + + test('refuses an absolute member', () => { + const tar = join(workDir, 'abs.tar.gz'); + writeTarball(tar, [{ name: '/etc/cron.d/pwned', content: 'x' }]); + + expect(() => extractTarball(tar, join(workDir, 'out'), 'abs-repo')) + .toThrow(/abs-repo.*absolute path/s); + }); + + test('refuses a symlink member', () => { + const tar = join(workDir, 'link.tar.gz'); + writeTarball(tar, [ + { name: 'index.html', content: 'ok' }, + { name: 'secrets.html', typeflag: '2', linkname: '/etc/passwd' }, + ]); + + // The traversal check passes (the name is innocuous); the post-extraction + // lstat walk is what catches this one. + expect(() => extractTarball(tar, join(workDir, 'out'), 'link-repo')) + .toThrow(/link-repo.*symlink/s); + }); + + test('refuses an empty archive', () => { + const tar = join(workDir, 'empty.tar.gz'); + writeTarball(tar, []); + + expect(() => extractTarball(tar, join(workDir, 'out'), 'empty-repo')) + .toThrow(/empty-repo.*empty/s); + }); +}); + +test.describe('copyDir', () => { + test('copies a tree without shelling out to cp', () => { + const src = join(workDir, 'src'); + mkdirSync(join(src, 'nested'), { recursive: true }); + writeFileSync(join(src, 'a.txt'), 'a'); + writeFileSync(join(src, 'nested', 'b.txt'), 'b'); + + const dest = join(workDir, 'dest'); + copyDir(src, dest); + + expect(readFileSync(join(dest, 'a.txt'), 'utf8')).toBe('a'); + expect(readFileSync(join(dest, 'nested', 'b.txt'), 'utf8')).toBe('b'); + }); +});