diff --git a/README.md b/README.md index f96bf82..a0389b6 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@
-markdown-code +markdown-code [![CI Build](https://github.com/scalvert/markdown-code/actions/workflows/ci.yml/badge.svg)](https://github.com/scalvert/markdown-code/actions/workflows/ci.yml) [![npm version](https://badge.fury.io/js/markdown-code.svg)](https://badge.fury.io/js/markdown-code) @@ -200,7 +200,8 @@ syntax as a local path: ```` HTTPS is allowed by default. Plain HTTP is rejected unless `allowInsecureHttp` is -set to `true`. Remote requests time out after 30 seconds by default; configure +set to `true`, including an HTTPS URL that redirects to HTTP. Remote files larger +than 5 MB are rejected. Remote requests time out after 30 seconds by default; configure `remoteTimeout` in milliseconds when needed. Remote failures remain errors even when `missingSnippetSeverity` is set to `warning`, because the source was configured but could not be loaded. diff --git a/package-lock.json b/package-lock.json index b1137ec..7c961ec 100644 --- a/package-lock.json +++ b/package-lock.json @@ -9,10 +9,8 @@ "version": "1.2.0", "license": "MIT", "dependencies": { - "@types/mdast": "^4.0.3", "fast-glob": "^3.3.2", "language-map": "^1.5.0", - "mdast": "^3.0.0", "picocolors": "^1.1.1", "remark": "^15.0.1", "remark-frontmatter": "^5.0.0", @@ -29,6 +27,7 @@ "devDependencies": { "@eslint/js": "^9.35.0", "@release-it-plugins/lerna-changelog": "^9.0.0", + "@types/mdast": "^4.0.4", "@types/node": "^25.6.0", "@types/yargs": "^17.0.35", "@typescript-eslint/eslint-plugin": "^8.58.1", @@ -8066,13 +8065,6 @@ "node": "*" } }, - "node_modules/mdast": { - "version": "3.0.0", - "resolved": "https://registry.npmjs.org/mdast/-/mdast-3.0.0.tgz", - "integrity": "sha512-xySmf8g4fPKMeC07jXGz971EkLbWAJ83s4US2Tj9lEdnZ142UP5grN73H1Xd3HzrdbU5o9GYYP/y8F9ZSwLE9g==", - "deprecated": "`mdast` was renamed to `remark`", - "license": "MIT" - }, "node_modules/mdast-util-from-markdown": { "version": "2.0.3", "resolved": "https://registry.npmjs.org/mdast-util-from-markdown/-/mdast-util-from-markdown-2.0.3.tgz", diff --git a/package.json b/package.json index 3784a6c..8262d34 100644 --- a/package.json +++ b/package.json @@ -22,8 +22,7 @@ "markdown-code": "dist/cli.js" }, "files": [ - "dist", - "markdown-code.png" + "dist" ], "scripts": { "build": "tsup", @@ -40,10 +39,8 @@ "typecheck": "tsc --noEmit" }, "dependencies": { - "@types/mdast": "^4.0.3", "fast-glob": "^3.3.2", "language-map": "^1.5.0", - "mdast": "^3.0.0", "picocolors": "^1.1.1", "remark": "^15.0.1", "remark-frontmatter": "^5.0.0", @@ -56,6 +53,7 @@ "devDependencies": { "@eslint/js": "^9.35.0", "@release-it-plugins/lerna-changelog": "^9.0.0", + "@types/mdast": "^4.0.4", "@types/node": "^25.6.0", "@types/yargs": "^17.0.35", "@typescript-eslint/eslint-plugin": "^8.58.1", diff --git a/scripts/smoke-test-package.mjs b/scripts/smoke-test-package.mjs index 217ae8c..666f63a 100644 --- a/scripts/smoke-test-package.mjs +++ b/scripts/smoke-test-package.mjs @@ -15,12 +15,7 @@ const repoRoot = resolve(import.meta.dirname, '..'); const { version } = JSON.parse( readFileSync(join(repoRoot, 'package.json'), 'utf-8'), ); -const allowedFiles = new Set([ - 'LICENSE', - 'README.md', - 'package.json', - 'markdown-code.png', -]); +const allowedFiles = new Set(['LICENSE', 'README.md', 'package.json']); const npmCli = process.env.npm_execpath; if (!npmCli) { diff --git a/src/glob.ts b/src/glob.ts index 45ffe24..f3bfd7d 100644 --- a/src/glob.ts +++ b/src/glob.ts @@ -24,5 +24,5 @@ export async function findMarkdownFiles({ cwd: workingDir, absolute: true, }); - return files.map((file) => resolve(file)); + return files.map((file) => resolve(file)).sort(); } diff --git a/src/remote.ts b/src/remote.ts index aef1837..4ae8353 100644 --- a/src/remote.ts +++ b/src/remote.ts @@ -1,9 +1,49 @@ export interface FetchOptions { timeout?: number | undefined; allowInsecureHttp?: boolean | undefined; + maxBytes?: number | undefined; } const DEFAULT_TIMEOUT = 30000; // 30 seconds +const DEFAULT_MAX_BYTES = 5 * 1024 * 1024; + +function tooLarge(url: string, maxBytes: number): Error { + return new Error(`Remote file is larger than ${maxBytes} bytes: ${url}`); +} + +async function readBody( + response: Response, + url: string, + maxBytes: number, +): Promise { + const declaredLength = Number(response.headers.get('content-length')); + if (declaredLength > maxBytes) { + await response.body?.cancel(); + throw tooLarge(url, maxBytes); + } + + if (!response.body) { + return ''; + } + + const reader = response.body.getReader(); + const decoder = new TextDecoder(); + let received = 0; + let text = ''; + + for (;;) { + const { done, value } = await reader.read(); + if (done) { + return text + decoder.decode(); + } + received += value.byteLength; + if (received > maxBytes) { + await reader.cancel(); + throw tooLarge(url, maxBytes); + } + text += decoder.decode(value, { stream: true }); + } +} /** * Check if a path is a remote URL (HTTP or HTTPS) @@ -77,6 +117,70 @@ export function validateRemoteUrl( /** * Fetch content from a remote URL. */ +const MAX_REDIRECTS = 10; +const REDIRECT_STATUSES = new Set([301, 302, 303, 307, 308]); + +function redirectTarget( + url: string, + from: string, + location: string, + options: FetchOptions, +): string { + if (!URL.canParse(location, from)) { + throw new Error(`Remote file sent an invalid redirect: ${url}`); + } + const target = new URL(location, from); + target.hash = ''; + + if (target.username || target.password) { + throw new Error(`Remote file redirected to a URL with credentials: ${url}`); + } + + if (target.protocol === 'http:' && !options.allowInsecureHttp) { + throw new Error( + `Remote file redirected to an insecure HTTP URL: ${url} -> ${target}. ` + + 'Use HTTPS or set allowInsecureHttp: true in config.', + ); + } + if (target.protocol !== 'http:' && target.protocol !== 'https:') { + throw new Error( + `Remote file redirected to an unsupported URL: ${url} -> ${target}`, + ); + } + return target.toString(); +} + +async function fetchFollowingRedirects( + url: string, + fetchUrl: string, + options: FetchOptions, + signal: AbortSignal, +): Promise { + let current = fetchUrl; + + for (let redirects = 0; ; redirects++) { + const response = await fetch(current, { + signal, + redirect: 'manual', + headers: { + 'User-Agent': 'markdown-code', + Accept: 'text/plain, */*', + }, + }); + + const location = response.headers.get('location'); + if (!REDIRECT_STATUSES.has(response.status) || location === null) { + return response; + } + + await response.body?.cancel(); + if (redirects === MAX_REDIRECTS) { + throw new Error(`Too many redirects: ${url}`); + } + current = redirectTarget(url, current, location, options); + } +} + export async function fetchRemoteContent( url: string, options: FetchOptions = {}, @@ -94,15 +198,15 @@ export async function fetchRemoteContent( const timeoutId = setTimeout(() => controller.abort(), timeout); try { - const response = await fetch(fetchUrl, { - signal: controller.signal, - headers: { - 'User-Agent': 'markdown-code', - Accept: 'text/plain, */*', - }, - }); + const response = await fetchFollowingRedirects( + url, + fetchUrl, + options, + controller.signal, + ); if (!response.ok) { + await response.body?.cancel(); if (response.status === 404) { throw new Error(`Remote file not found: ${url}`); } @@ -111,7 +215,7 @@ export async function fetchRemoteContent( ); } - return await response.text(); + return await readBody(response, url, options.maxBytes ?? DEFAULT_MAX_BYTES); } catch (error) { if (error instanceof Error) { if (error.name === 'AbortError') { diff --git a/test/glob.test.ts b/test/glob.test.ts index b825129..0b4aaa9 100644 --- a/test/glob.test.ts +++ b/test/glob.test.ts @@ -1,3 +1,4 @@ +import { mkdirSync, symlinkSync, writeFileSync } from 'node:fs'; import { join } from 'node:path'; import { afterEach, beforeEach, describe, expect, it } from 'vitest'; import { Project } from 'fixturify-project'; @@ -50,6 +51,49 @@ describe('findMarkdownFiles', () => { ); }); + it('includes the working directory itself in a ../ pattern', async () => { + const files = await findMarkdownFiles({ + markdownGlob: '../**/*.md', + excludeGlob: [], + workingDir: join(project.baseDir, 'docs'), + }); + + expect(files).toEqual([ + join(project.baseDir, 'README.md'), + join(project.baseDir, 'docs', 'draft.md'), + join(project.baseDir, 'docs', 'guide.md'), + ]); + }); + + it('follows symlinked directories under a symlinked absolute root', async () => { + const outside = new Project(); + await outside.write({ 'linked.md': '' }); + try { + mkdirSync(join(project.baseDir, 'tree')); + writeFileSync(join(project.baseDir, 'tree', 'own.md'), ''); + symlinkSync( + outside.baseDir, + join(project.baseDir, 'tree', 'link'), + 'junction', + ); + const alias = join(project.baseDir, 'alias'); + symlinkSync(join(project.baseDir, 'tree'), alias, 'junction'); + + const files = await findMarkdownFiles({ + markdownGlob: join(alias, '**', '*.md'), + excludeGlob: [], + workingDir: alias, + }); + + expect(files).toEqual([ + join(alias, 'link', 'linked.md'), + join(alias, 'own.md'), + ]); + } finally { + outside.dispose(); + } + }); + it('accepts an absolute pattern built with platform paths', async () => { const files = await findMarkdownFiles({ markdownGlob: join(project.baseDir, 'docs', '*.md'), diff --git a/test/remote.test.ts b/test/remote.test.ts index cf109ef..b30c4aa 100644 --- a/test/remote.test.ts +++ b/test/remote.test.ts @@ -138,20 +138,14 @@ describe('remote', () => { it('should fetch content from valid URL', async () => { const mockContent = 'const x = 1;'; - vi.mocked(fetch).mockResolvedValue({ - ok: true, - text: () => Promise.resolve(mockContent), - } as Response); + vi.mocked(fetch).mockResolvedValue(new Response(mockContent)); const result = await fetchRemoteContent('https://example.com/file.ts'); expect(result).toBe(mockContent); }); it('should strip hash fragment when fetching', async () => { - vi.mocked(fetch).mockResolvedValue({ - ok: true, - text: () => Promise.resolve('content'), - } as Response); + vi.mocked(fetch).mockResolvedValue(new Response('content')); await fetchRemoteContent('https://example.com/file.ts#L10-L20'); @@ -163,21 +157,30 @@ describe('remote', () => { }); it('should throw on 404', async () => { - vi.mocked(fetch).mockResolvedValue({ - ok: false, - status: 404, - } as Response); + vi.mocked(fetch).mockResolvedValue(new Response(null, { status: 404 })); await expect( fetchRemoteContent('https://example.com/missing.ts'), ).rejects.toThrow('Remote file not found'); }); + it('cancels the body of an error response', async () => { + let cancelled = false; + const body = new ReadableStream({ + cancel() { + cancelled = true; + }, + }); + vi.mocked(fetch).mockResolvedValue(new Response(body, { status: 500 })); + + await expect( + fetchRemoteContent('https://example.com/file.ts'), + ).rejects.toThrow('HTTP 500'); + expect(cancelled).toBe(true); + }); + it('should throw on other HTTP errors', async () => { - vi.mocked(fetch).mockResolvedValue({ - ok: false, - status: 500, - } as Response); + vi.mocked(fetch).mockResolvedValue(new Response(null, { status: 500 })); await expect( fetchRemoteContent('https://example.com/file.ts'), @@ -185,10 +188,7 @@ describe('remote', () => { }); it('should include User-Agent header', async () => { - vi.mocked(fetch).mockResolvedValue({ - ok: true, - text: () => Promise.resolve('content'), - } as Response); + vi.mocked(fetch).mockResolvedValue(new Response('content')); await fetchRemoteContent('https://example.com/file.ts'); @@ -208,11 +208,188 @@ describe('remote', () => { ).rejects.toThrow('Insecure HTTP URLs are not allowed'); }); + describe('redirects', () => { + function redirect(location: string, status = 302) { + return new Response('moved', { status, headers: { location } }); + } + + function requestedUrls() { + return vi.mocked(fetch).mock.calls.map(([input]) => String(input)); + } + + it('rejects a redirect to HTTP without requesting it', async () => { + vi.mocked(fetch).mockResolvedValueOnce( + redirect('http://example.com/file.ts'), + ); + + await expect( + fetchRemoteContent('https://example.com/file.ts'), + ).rejects.toThrow( + 'Remote file redirected to an insecure HTTP URL: https://example.com/file.ts -> http://example.com/file.ts', + ); + expect(requestedUrls()).toEqual(['https://example.com/file.ts']); + }); + + it('rejects an HTTP hop in the middle of a redirect chain', async () => { + vi.mocked(fetch) + .mockResolvedValueOnce(redirect('https://a.example.com/one')) + .mockResolvedValueOnce(redirect('http://b.example.com/two')); + + await expect( + fetchRemoteContent('https://example.com/file.ts'), + ).rejects.toThrow('-> http://b.example.com/two'); + expect(requestedUrls()).toEqual([ + 'https://example.com/file.ts', + 'https://a.example.com/one', + ]); + }); + + it('follows HTTP hops when allowInsecureHttp is true', async () => { + vi.mocked(fetch) + .mockResolvedValueOnce(redirect('http://example.com/two', 301)) + .mockResolvedValueOnce(redirect('https://example.com/three', 308)) + .mockResolvedValueOnce(new Response('content')); + + await expect( + fetchRemoteContent('https://example.com/file.ts', { + allowInsecureHttp: true, + }), + ).resolves.toBe('content'); + expect(requestedUrls()).toEqual([ + 'https://example.com/file.ts', + 'http://example.com/two', + 'https://example.com/three', + ]); + }); + + it('resolves a relative Location against the current URL', async () => { + vi.mocked(fetch) + .mockResolvedValueOnce(redirect('../raw/file.ts#L1-L2', 307)) + .mockResolvedValueOnce(new Response('content')); + + await expect( + fetchRemoteContent('https://example.com/blob/file.ts'), + ).resolves.toBe('content'); + expect(requestedUrls()).toEqual([ + 'https://example.com/blob/file.ts', + 'https://example.com/raw/file.ts', + ]); + }); + + it('rejects a redirect to a non-HTTP scheme', async () => { + vi.mocked(fetch).mockResolvedValueOnce(redirect('file:///etc/passwd')); + + await expect( + fetchRemoteContent('https://example.com/file.ts'), + ).rejects.toThrow( + 'Remote file redirected to an unsupported URL: https://example.com/file.ts -> file:///etc/passwd', + ); + }); + + it('rejects a redirect to a URL with credentials without echoing them', async () => { + vi.mocked(fetch).mockResolvedValueOnce( + redirect('https://user:secret@example.com/file.ts'), + ); + + const error = await fetchRemoteContent( + 'https://example.com/file.ts', + ).catch((caught: unknown) => caught as Error); + + expect(error.message).toBe( + 'Remote file redirected to a URL with credentials: https://example.com/file.ts', + ); + }); + + it('names the URL when the redirect Location is invalid', async () => { + vi.mocked(fetch).mockResolvedValueOnce(redirect('http://[::1')); + + await expect( + fetchRemoteContent('https://example.com/file.ts'), + ).rejects.toThrow( + 'Remote file sent an invalid redirect: https://example.com/file.ts', + ); + }); + + it('gives up after 10 redirects', async () => { + vi.mocked(fetch).mockImplementation(() => + Promise.resolve(redirect('https://example.com/again')), + ); + + await expect( + fetchRemoteContent('https://example.com/file.ts'), + ).rejects.toThrow('Too many redirects: https://example.com/file.ts'); + expect(requestedUrls()).toHaveLength(11); + }); + + it('treats a 3xx without a Location as the final response', async () => { + vi.mocked(fetch).mockResolvedValueOnce( + new Response(null, { status: 304 }), + ); + + await expect( + fetchRemoteContent('https://example.com/file.ts'), + ).rejects.toThrow('HTTP 304'); + }); + }); + + describe('size limit', () => { + function streamOf(chunks: Array): ReadableStream { + return new ReadableStream({ + start(controller) { + chunks.forEach((chunk) => controller.enqueue(chunk)); + controller.close(); + }, + }); + } + + it('rejects a declared Content-Length over the limit', async () => { + vi.mocked(fetch).mockResolvedValue( + new Response('x', { headers: { 'content-length': '11' } }), + ); + + await expect( + fetchRemoteContent('https://example.com/big.ts', { maxBytes: 10 }), + ).rejects.toThrow( + 'Remote file is larger than 10 bytes: https://example.com/big.ts', + ); + }); + + it('stops reading a body that grows past the limit', async () => { + vi.mocked(fetch).mockResolvedValue( + new Response( + streamOf([new Uint8Array(6), new Uint8Array(6), new Uint8Array(6)]), + ), + ); + + await expect( + fetchRemoteContent('https://example.com/big.ts', { maxBytes: 10 }), + ).rejects.toThrow('Remote file is larger than 10 bytes'); + }); + + it('accepts a body exactly at the limit', async () => { + vi.mocked(fetch).mockResolvedValue(new Response('0123456789')); + + await expect( + fetchRemoteContent('https://example.com/ok.ts', { maxBytes: 10 }), + ).resolves.toBe('0123456789'); + }); + + it('decodes UTF-8 characters split across chunks', async () => { + const bytes = new TextEncoder().encode('café → 🚀'); + vi.mocked(fetch).mockResolvedValue( + new Response( + streamOf([bytes.slice(0, 4), bytes.slice(4, 9), bytes.slice(9)]), + ), + ); + + await expect( + fetchRemoteContent('https://example.com/utf8.ts'), + ).resolves.toBe('café → 🚀'); + }); + }); + it('should allow HTTP when allowInsecureHttp is true', async () => { - vi.mocked(fetch).mockResolvedValue({ - ok: true, - text: () => Promise.resolve('content'), - } as Response); + vi.mocked(fetch).mockResolvedValue(new Response('content')); await expect( fetchRemoteContent('http://example.com/file.ts', { diff --git a/test/sync-plan.test.ts b/test/sync-plan.test.ts index 48bb1c4..bb8d9b9 100644 --- a/test/sync-plan.test.ts +++ b/test/sync-plan.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import { chmodSync, readFileSync, writeFileSync } from 'node:fs'; import { join } from 'node:path'; import { Project } from 'fixturify-project'; @@ -11,13 +11,6 @@ import { import { syncMarkdownFiles } from '../src/sync.js'; import type { RuntimeConfig } from '../src/types.js'; -vi.mock('fast-glob', async (importOriginal) => { - const actual = await importOriginal(); - const sortedGlob = async (...args: Parameters) => - (await actual.default(...args)).sort(); - return { default: sortedGlob }; -}); - async function collect(plan: SyncPlan): Promise> { if (plan.status !== 'planned') { throw new Error(`Expected a plan, got ${plan.error}`);