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.

package.json

@@ -39,11 +39,12 @@
     "lint:fix": "eslint . --ext .js,.cjs,.mjs,.yaml --fix",
     "lint:md": "markdownlint-cli2 \"**/*.md\"",
     "prepare": "command -v husky >/dev/null 2>&1 && husky || exit 0",
-    "quality": "npm run format:check && npm run lint && npm run lint:md && npm run docs:build && npm run test:install && npm run validate:refs && npm run validate:skills",
+    "quality": "npm run format:check && npm run lint && npm run lint:md && npm run docs:build && npm run test:install && npm run test:registry && npm run validate:refs && npm run validate:skills",
     "rebundle": "node tools/installer/bundlers/bundle-web.js rebundle",
-    "test": "npm run test:refs && npm run test:install && npm run lint && npm run lint:md && npm run format:check",
+    "test": "npm run test:refs && npm run test:install && npm run test:registry && npm run lint && npm run lint:md && npm run format:check",
     "test:install": "node test/test-installation-components.js",
     "test:refs": "node test/test-file-refs-csv.js",
+    "test:registry": "node test/test-registry-client.js",
     "validate:refs": "node tools/validate-file-refs.js --strict",
     "validate:skills": "node tools/validate-skills.js --strict"
   },

test/test-registry-client.js

@@ -0,0 +1,213 @@
+/**
+ * RegistryClient Tests
+ *
+ * Tests the GitHub API cascade logic in RegistryClient:
+ * - fetchGitHubFile tries API first, falls back to raw CDN
+ * - fetchGitHubYaml/Json parse correctly
+ * - Error propagation when both endpoints fail
+ *
+ * Uses monkey-patching to intercept HTTP calls without external dependencies.
+ * Usage: node test/test-registry-client.js
+ */
+
+const { RegistryClient } = require('../tools/installer/modules/registry-client');
+
+// ANSI colors
+const colors = {
+  reset: '\u001B[0m',
+  green: '\u001B[32m',
+  red: '\u001B[31m',
+  cyan: '\u001B[36m',
+  dim: '\u001B[2m',
+};
+
+let passed = 0;
+let failed = 0;
+
+function assert(condition, testName, errorMessage = '') {
+  if (condition) {
+    console.log(`${colors.green}✓${colors.reset} ${testName}`);
+    passed++;
+  } else {
+    console.log(`${colors.red}✗${colors.reset} ${testName}`);
+    if (errorMessage) {
+      console.log(`  ${colors.dim}${errorMessage}${colors.reset}`);
+    }
+    failed++;
+  }
+}
+
+// ─── Test Helpers ──────────────────────────────────────────────────────────
+
+/**
+ * Create a RegistryClient with stubbed fetch methods for testing cascade logic.
+ * @param {Object} opts
+ * @param {string|Error} opts.apiResult - Return value or Error to throw for API call
+ * @param {string|Error} opts.rawResult - Return value or Error to throw for raw CDN call
+ * @returns {{ client: RegistryClient, calls: string[] }}
+ */
+function createStubbedClient({ apiResult, rawResult }) {
+  const client = new RegistryClient();
+  const calls = [];
+
+  // Stub _fetchWithHeaders (GitHub API path)
+  client._fetchWithHeaders = async (url) => {
+    calls.push(`api:${url}`);
+    if (apiResult instanceof Error) throw apiResult;
+    return apiResult;
+  };
+
+  // Stub fetch (raw CDN path)
+  const originalFetch = client.fetch.bind(client);
+  client.fetch = async (url, timeout) => {
+    // Only intercept raw.githubusercontent.com calls
+    if (url.includes('raw.githubusercontent.com')) {
+      calls.push(`raw:${url}`);
+      if (rawResult instanceof Error) throw rawResult;
+      return rawResult;
+    }
+    return originalFetch(url, timeout);
+  };
+
+  return { client, calls };
+}
+
+// ─── Tests ─────────────────────────────────────────────────────────────────
+
+async function testApiSuccessSkipsRaw() {
+  const { client, calls } = createStubbedClient({
+    apiResult: 'api-content',
+    rawResult: 'raw-content',
+  });
+
+  const result = await client.fetchGitHubFile('owner', 'repo', 'path/file.txt', 'main');
+
+  assert(result === 'api-content', 'API success returns API content');
+  assert(calls.length === 1, 'API success makes exactly one call', `calls: ${calls.join(', ')}`);
+  assert(calls[0].startsWith('api:'), 'API success calls API endpoint', `got: ${calls[0]}`);
+}
+
+async function testApiFailureFallsToRaw() {
+  const { client, calls } = createStubbedClient({
+    apiResult: new Error('HTTP 403'),
+    rawResult: 'raw-content',
+  });
+
+  const result = await client.fetchGitHubFile('owner', 'repo', 'path/file.txt', 'main');
+
+  assert(result === 'raw-content', 'API failure returns raw CDN content');
+  assert(calls.length === 2, 'API failure makes two calls', `calls: ${calls.join(', ')}`);
+  assert(calls[0].startsWith('api:'), 'First call is to API');
+  assert(calls[1].startsWith('raw:'), 'Second call is to raw CDN');
+}
+
+async function testBothFailThrows() {
+  const { client } = createStubbedClient({
+    apiResult: new Error('HTTP 403'),
+    rawResult: new Error('HTTP 404'),
+  });
+
+  let threw = false;
+  try {
+    await client.fetchGitHubFile('owner', 'repo', 'path/file.txt', 'main');
+  } catch {
+    threw = true;
+  }
+
+  assert(threw, 'Both endpoints failing throws an error');
+}
+
+async function testUrlConstruction() {
+  const { client, calls } = createStubbedClient({
+    apiResult: 'content',
+    rawResult: 'content',
+  });
+
+  await client.fetchGitHubFile('bmad-code-org', 'bmad-plugins-marketplace', 'registry/official.yaml', 'main');
+
+  const apiCall = calls[0];
+  assert(
+    apiCall.includes('api.github.com/repos/bmad-code-org/bmad-plugins-marketplace/contents/registry/official.yaml'),
+    'API URL contains correct path',
+    `got: ${apiCall}`,
+  );
+  assert(apiCall.includes('ref=main'), 'API URL contains ref parameter', `got: ${apiCall}`);
+}
+
+async function testRawUrlConstruction() {
+  const { client, calls } = createStubbedClient({
+    apiResult: new Error('fail'),
+    rawResult: 'content',
+  });
+
+  await client.fetchGitHubFile('bmad-code-org', 'bmad-plugins-marketplace', 'registry/official.yaml', 'main');
+
+  const rawCall = calls[1];
+  assert(
+    rawCall.includes('raw.githubusercontent.com/bmad-code-org/bmad-plugins-marketplace/main/registry/official.yaml'),
+    'Raw URL contains correct path',
+    `got: ${rawCall}`,
+  );
+}
+
+async function testFetchGitHubYamlParsesCorrectly() {
+  const yamlContent = 'modules:\n  - name: test\n    description: A test module\n';
+  const { client } = createStubbedClient({
+    apiResult: yamlContent,
+    rawResult: yamlContent,
+  });
+
+  const result = await client.fetchGitHubYaml('owner', 'repo', 'file.yaml', 'main');
+
+  assert(Array.isArray(result.modules), 'fetchGitHubYaml parses YAML correctly');
+  assert(result.modules[0].name === 'test', 'fetchGitHubYaml preserves YAML values');
+}
+
+async function testFetchGitHubJsonParsesCorrectly() {
+  const jsonContent = '{"name": "test", "version": "1.0.0"}';
+  const { client } = createStubbedClient({
+    apiResult: jsonContent,
+    rawResult: jsonContent,
+  });
+
+  const result = await client.fetchGitHubJson('owner', 'repo', 'file.json', 'main');
+
+  assert(result.name === 'test', 'fetchGitHubJson parses JSON correctly');
+  assert(result.version === '1.0.0', 'fetchGitHubJson preserves JSON values');
+}
+
+// ─── Runner ────────────────────────────────────────────────────────────────
+
+async function runTests() {
+  console.log(`\n${colors.cyan}========================================`);
+  console.log('  RegistryClient Tests');
+  console.log(`========================================${colors.reset}\n`);
+
+  await testApiSuccessSkipsRaw();
+  await testApiFailureFallsToRaw();
+  await testBothFailThrows();
+  await testUrlConstruction();
+  await testRawUrlConstruction();
+  await testFetchGitHubYamlParsesCorrectly();
+  await testFetchGitHubJsonParsesCorrectly();
+
+  console.log(`\n${colors.cyan}========================================`);
+  console.log('Test Results:');
+  console.log(`  Passed: ${colors.green}${passed}${colors.reset}`);
+  console.log(`  Failed: ${colors.red}${failed}${colors.reset}`);
+  console.log(`========================================${colors.reset}\n`);
+
+  if (failed === 0) {
+    console.log(`${colors.green}✨ All registry client tests passed!${colors.reset}\n`);
+    process.exit(0);
+  } else {
+    console.log(`${colors.red}❌ Some registry client tests failed${colors.reset}\n`);
+    process.exit(1);
+  }
+}
+
+runTests().catch((error) => {
+  console.error(`${colors.red}Test runner failed:${colors.reset}`, error.message);
+  console.error(error.stack);
+  process.exit(1);
+});

tools/installer/modules/community-manager.js

@@ -5,9 +5,9 @@ const { execSync } = require('node:child_process');
 const prompts = require('../prompts');
 const { RegistryClient } = require('./registry-client');
 
-const MARKETPLACE_BASE = 'https://raw.githubusercontent.com/bmad-code-org/bmad-plugins-marketplace/main';
-const COMMUNITY_INDEX_URL = `${MARKETPLACE_BASE}/registry/community-index.yaml`;
-const CATEGORIES_URL = `${MARKETPLACE_BASE}/categories.yaml`;
+const MARKETPLACE_OWNER = 'bmad-code-org';
+const MARKETPLACE_REPO = 'bmad-plugins-marketplace';
+const MARKETPLACE_REF = 'main';
 
 /**
  * Manages community modules from the BMad marketplace registry.
@@ -33,7 +33,12 @@ class CommunityModuleManager {
     if (this._cachedIndex) return this._cachedIndex;
 
     try {
-      const config = await this._client.fetchYaml(COMMUNITY_INDEX_URL);
+      const config = await this._client.fetchGitHubYaml(
+        MARKETPLACE_OWNER,
+        MARKETPLACE_REPO,
+        'registry/community-index.yaml',
+        MARKETPLACE_REF,
+      );
       if (config?.modules?.length) {
         this._cachedIndex = config;
         return config;
@@ -54,7 +59,7 @@ class CommunityModuleManager {
     if (this._cachedCategories) return this._cachedCategories;
 
     try {
-      const config = await this._client.fetchYaml(CATEGORIES_URL);
+      const config = await this._client.fetchGitHubYaml(MARKETPLACE_OWNER, MARKETPLACE_REPO, 'categories.yaml', MARKETPLACE_REF);
       if (config?.categories) {
         this._cachedCategories = config;
         return config;

tools/installer/modules/external-manager.js

@@ -6,7 +6,9 @@ const yaml = require('yaml');
 const prompts = require('../prompts');
 const { RegistryClient } = require('./registry-client');
 
-const REGISTRY_RAW_URL = 'https://raw.githubusercontent.com/bmad-code-org/bmad-plugins-marketplace/main/registry/official.yaml';
+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');
 
 /**
@@ -33,8 +35,7 @@ class ExternalModuleManager {
 
     // Try remote registry first
     try {
-      const content = await this._client.fetch(REGISTRY_RAW_URL);
-      const config = yaml.parse(content);
+      const config = await this._client.fetchGitHubYaml(MARKETPLACE_OWNER, MARKETPLACE_REPO, 'registry/official.yaml', MARKETPLACE_REF);
       if (config?.modules?.length) {
         this.cachedModules = config;
         return config;

tools/installer/modules/registry-client.js

@@ -12,18 +12,22 @@ class RegistryClient {
 
   /**
    * 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 {number} [timeout] - Timeout in ms (overrides default)
+   * @param {number} [maxRedirects=3] - Maximum redirects to follow
    * @returns {Promise<string>} Response body
    */
-  fetch(url, timeout) {
+  fetch(url, timeout, maxRedirects = 3) {
     const timeoutMs = timeout || this.timeout;
     return new Promise((resolve, reject) => {
       const req = https
         .get(url, { timeout: timeoutMs }, (res) => {
           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) {
             return reject(new Error(`HTTP ${res.statusCode}`));
@@ -61,6 +65,113 @@ class RegistryClient {
     const content = await this.fetch(url, timeout);
     return JSON.parse(content);
   }
+
+  /**
+   * Fetch a file from a GitHub repo using the Contents API first,
+   * falling back to raw.githubusercontent.com if the API fails.
+   *
+   * The API endpoint (`api.github.com`) is tried first because corporate
+   * proxies commonly block `raw.githubusercontent.com` while allowing
+   * `api.github.com` under the "Software Development" category.
+   *
+   * @param {string} owner - Repository owner (e.g., 'bmad-code-org')
+   * @param {string} repo  - Repository name (e.g., 'bmad-plugins-marketplace')
+   * @param {string} filePath - Path within the repo (e.g., 'registry/official.yaml')
+   * @param {string} ref   - Git ref (branch, tag, or SHA; e.g., 'main')
+   * @param {number} [timeout] - Timeout in ms (overrides default)
+   * @returns {Promise<string>} Raw file content
+   */
+  async fetchGitHubFile(owner, repo, filePath, ref, timeout) {
+    const apiUrl = `https://api.github.com/repos/${owner}/${repo}/contents/${filePath}?ref=${ref}`;
+    const rawUrl = `https://raw.githubusercontent.com/${owner}/${repo}/${ref}/${filePath}`;
+
+    // Try GitHub Contents API first (with raw content accept header)
+    try {
+      return await this._fetchWithHeaders(apiUrl, { Accept: 'application/vnd.github.raw+json' }, timeout);
+    } catch (apiError) {
+      // 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}`);
+      }
+    }
+  }
+
+  /**
+   * Fetch a file from GitHub and parse as YAML.
+   * @param {string} owner - Repository owner
+   * @param {string} repo  - Repository name
+   * @param {string} filePath - Path within the repo
+   * @param {string} ref   - Git ref
+   * @param {number} [timeout] - Timeout in ms
+   * @returns {Promise<Object>} Parsed YAML content
+   */
+  async fetchGitHubYaml(owner, repo, filePath, ref, timeout) {
+    const content = await this.fetchGitHubFile(owner, repo, filePath, ref, timeout);
+    return yaml.parse(content);
+  }
+
+  /**
+   * Fetch a file from GitHub and parse as JSON.
+   * @param {string} owner - Repository owner
+   * @param {string} repo  - Repository name
+   * @param {string} filePath - Path within the repo
+   * @param {string} ref   - Git ref
+   * @param {number} [timeout] - Timeout in ms
+   * @returns {Promise<Object>} Parsed JSON content
+   */
+  async fetchGitHubJson(owner, repo, filePath, ref, timeout) {
+    const content = await this.fetchGitHubFile(owner, repo, filePath, ref, timeout);
+    return JSON.parse(content);
+  }
+
+  /**
+   * Fetch a URL with custom headers. Used for GitHub API requests.
+   * Follows up to 3 redirects.
+   * @param {string} url - URL to fetch
+   * @param {Object} headers - Request headers
+   * @param {number} [timeout] - Timeout in ms
+   * @param {number} [maxRedirects=3] - Maximum redirects to follow
+   * @returns {Promise<string>} Response body
+   * @private
+   */
+  _fetchWithHeaders(url, headers, timeout, maxRedirects = 3) {
+    const timeoutMs = timeout || this.timeout;
+    const parsed = new URL(url);
+    const options = {
+      hostname: parsed.hostname,
+      path: parsed.pathname + parsed.search,
+      timeout: timeoutMs,
+      headers: {
+        'User-Agent': 'bmad-installer',
+        ...headers,
+      },
+    };
+
+    return new Promise((resolve, reject) => {
+      const req = https
+        .get(options, (res) => {
+          if (res.statusCode >= 300 && res.statusCode < 400 && res.headers.location) {
+            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) {
+            return reject(new Error(`HTTP ${res.statusCode}`));
+          }
+          let data = '';
+          res.on('data', (chunk) => (data += chunk));
+          res.on('end', () => resolve(data));
+        })
+        .on('error', reject)
+        .on('timeout', () => {
+          req.destroy();
+          reject(new Error('Request timed out'));
+        });
+    });
+  }
 }
 
 module.exports = { RegistryClient };