fix(installer): cap redirect depth and preserve dual-fallback errors

Add maxRedirects parameter to fetch() and _fetchWithHeaders() to prevent
unbounded redirect recursion. Wrap CDN fallback in try/catch and throw
AggregateError with both API and CDN errors for better diagnostics.
Extract marketplace repo coordinates into named constants in
external-manager.
This commit is contained in:
Alex Verkhovsky 2026-04-12 14:47:17 -07:00
parent b24b3402c2
commit 8133e57d57
2 changed files with 24 additions and 10 deletions

View File

@ -6,6 +6,9 @@ const yaml = require('yaml');
const prompts = require('../prompts'); const prompts = require('../prompts');
const { RegistryClient } = require('./registry-client'); const { RegistryClient } = require('./registry-client');
const MARKETPLACE_OWNER = 'bmad-code-org';
const MARKETPLACE_REPO = 'bmad-plugins-marketplace';
const MARKETPLACE_REF = 'main';
const FALLBACK_CONFIG_PATH = path.join(__dirname, 'registry-fallback.yaml'); const FALLBACK_CONFIG_PATH = path.join(__dirname, 'registry-fallback.yaml');
/** /**
@ -32,7 +35,7 @@ class ExternalModuleManager {
// Try remote registry first // Try remote registry first
try { try {
const config = await this._client.fetchGitHubYaml('bmad-code-org', 'bmad-plugins-marketplace', 'registry/official.yaml', 'main'); const config = await this._client.fetchGitHubYaml(MARKETPLACE_OWNER, MARKETPLACE_REPO, 'registry/official.yaml', MARKETPLACE_REF);
if (config?.modules?.length) { if (config?.modules?.length) {
this.cachedModules = config; this.cachedModules = config;
return config; return config;

View File

@ -12,18 +12,22 @@ class RegistryClient {
/** /**
* Fetch a URL and return the response body as a string. * Fetch a URL and return the response body as a string.
* Follows one redirect (GitHub sometimes 301s). * Follows up to 3 redirects (GitHub sometimes 301s).
* @param {string} url - URL to fetch * @param {string} url - URL to fetch
* @param {number} [timeout] - Timeout in ms (overrides default) * @param {number} [timeout] - Timeout in ms (overrides default)
* @param {number} [maxRedirects=3] - Maximum redirects to follow
* @returns {Promise<string>} Response body * @returns {Promise<string>} Response body
*/ */
fetch(url, timeout) { fetch(url, timeout, maxRedirects = 3) {
const timeoutMs = timeout || this.timeout; const timeoutMs = timeout || this.timeout;
return new Promise((resolve, reject) => { return new Promise((resolve, reject) => {
const req = https const req = https
.get(url, { timeout: timeoutMs }, (res) => { .get(url, { timeout: timeoutMs }, (res) => {
if (res.statusCode >= 300 && res.statusCode < 400 && res.headers.location) { if (res.statusCode >= 300 && res.statusCode < 400 && res.headers.location) {
return this.fetch(res.headers.location, timeoutMs).then(resolve, reject); if (maxRedirects <= 0) {
return reject(new Error('Too many redirects'));
}
return this.fetch(res.headers.location, timeoutMs, maxRedirects - 1).then(resolve, reject);
} }
if (res.statusCode !== 200) { if (res.statusCode !== 200) {
return reject(new Error(`HTTP ${res.statusCode}`)); return reject(new Error(`HTTP ${res.statusCode}`));
@ -84,11 +88,14 @@ class RegistryClient {
// Try GitHub Contents API first (with raw content accept header) // Try GitHub Contents API first (with raw content accept header)
try { try {
return await this._fetchWithHeaders(apiUrl, { Accept: 'application/vnd.github.raw+json' }, timeout); return await this._fetchWithHeaders(apiUrl, { Accept: 'application/vnd.github.raw+json' }, timeout);
} catch { } catch (apiError) {
// API failed — fall back to raw CDN // API failed — fall back to raw CDN
try {
return await this.fetch(rawUrl, timeout);
} catch (cdnError) {
throw new AggregateError([apiError, cdnError], `Both GitHub API and raw CDN failed for ${filePath}`);
}
} }
return this.fetch(rawUrl, timeout);
} }
/** /**
@ -121,14 +128,15 @@ class RegistryClient {
/** /**
* Fetch a URL with custom headers. Used for GitHub API requests. * Fetch a URL with custom headers. Used for GitHub API requests.
* Follows one redirect. * Follows up to 3 redirects.
* @param {string} url - URL to fetch * @param {string} url - URL to fetch
* @param {Object} headers - Request headers * @param {Object} headers - Request headers
* @param {number} [timeout] - Timeout in ms * @param {number} [timeout] - Timeout in ms
* @param {number} [maxRedirects=3] - Maximum redirects to follow
* @returns {Promise<string>} Response body * @returns {Promise<string>} Response body
* @private * @private
*/ */
_fetchWithHeaders(url, headers, timeout) { _fetchWithHeaders(url, headers, timeout, maxRedirects = 3) {
const timeoutMs = timeout || this.timeout; const timeoutMs = timeout || this.timeout;
const parsed = new URL(url); const parsed = new URL(url);
const options = { const options = {
@ -145,7 +153,10 @@ class RegistryClient {
const req = https const req = https
.get(options, (res) => { .get(options, (res) => {
if (res.statusCode >= 300 && res.statusCode < 400 && res.headers.location) { if (res.statusCode >= 300 && res.statusCode < 400 && res.headers.location) {
return this._fetchWithHeaders(res.headers.location, headers, timeoutMs).then(resolve, reject); if (maxRedirects <= 0) {
return reject(new Error('Too many redirects'));
}
return this._fetchWithHeaders(res.headers.location, headers, timeoutMs, maxRedirects - 1).then(resolve, reject);
} }
if (res.statusCode !== 200) { if (res.statusCode !== 200) {
return reject(new Error(`HTTP ${res.statusCode}`)); return reject(new Error(`HTTP ${res.statusCode}`));