use GitHub App tokens for backend GitHub calls

backend/__tests__/unit/services/githubAppService.apiHeaders.test.js

@@ -0,0 +1,77 @@
+// Credential precedence for backend GitHub calls: the App installation token is
+// the default, and the shared GITHUB_PAT is used only when no App is configured.
+const GitHubAppService = require('../../../services/githubAppService');
+
+const ENV_KEYS = [
+  'GITHUB_APP_ID',
+  'GITHUB_APP_PRIVATE_KEY',
+  'GITHUB_APP_INSTALLATION_ID_COMMONLY',
+  'GITHUB_PAT',
+];
+
+describe('GitHubAppService._apiHeaders credential precedence', () => {
+  const saved = {};
+  let mint;
+  let warn;
+
+  beforeEach(() => {
+    ENV_KEYS.forEach((k) => { saved[k] = process.env[k]; delete process.env[k]; });
+    mint = jest.spyOn(GitHubAppService, 'getInstallationToken')
+      .mockResolvedValue({ token: 'app-installation-token', expiresAt: null });
+    warn = jest.spyOn(console, 'warn').mockImplementation(() => {});
+  });
+
+  afterEach(() => {
+    ENV_KEYS.forEach((k) => {
+      if (saved[k] === undefined) delete process.env[k]; else process.env[k] = saved[k];
+    });
+    jest.restoreAllMocks();
+  });
+
+  const configureApp = () => {
+    process.env.GITHUB_APP_ID = '1';
+    process.env.GITHUB_APP_PRIVATE_KEY = 'pem';
+    process.env.GITHUB_APP_INSTALLATION_ID_COMMONLY = '42';
+  };
+
+  it('uses the App installation token when both the App and GITHUB_PAT are configured', async () => {
+    configureApp();
+    process.env.GITHUB_PAT = 'shared-pat';
+    const headers = await GitHubAppService._apiHeaders();
+    expect(headers.Authorization).toBe('Bearer app-installation-token');
+    expect(mint).toHaveBeenCalledWith('42');
+    expect(warn).not.toHaveBeenCalled();
+  });
+
+  it('mints for an explicit installationId over the default one', async () => {
+    configureApp();
+    await GitHubAppService._apiHeaders('99');
+    expect(mint).toHaveBeenCalledWith('99');
+  });
+
+  it('falls back to GITHUB_PAT only when no App is configured, and logs it', async () => {
+    process.env.GITHUB_PAT = 'shared-pat';
+    const headers = await GitHubAppService._apiHeaders();
+    expect(headers.Authorization).toBe('Bearer shared-pat');
+    expect(mint).not.toHaveBeenCalled();
+    expect(warn).toHaveBeenCalledWith(expect.stringContaining('falling back to GITHUB_PAT'));
+  });
+
+  it('never falls back to the PAT for an app-only request', async () => {
+    process.env.GITHUB_PAT = 'shared-pat';
+    await expect(GitHubAppService._apiHeaders(undefined, true)).rejects.toThrow('github_not_configured');
+    await expect(GitHubAppService._apiHeaders('99')).rejects.toThrow('github_not_configured');
+    expect(mint).not.toHaveBeenCalled();
+  });
+
+  it('rejects an explicit empty installationId instead of using the default or the PAT', async () => {
+    configureApp();
+    process.env.GITHUB_PAT = 'shared-pat';
+    await expect(GitHubAppService._apiHeaders('')).rejects.toThrow('github_not_configured');
+    expect(mint).not.toHaveBeenCalled();
+  });
+
+  it('throws when nothing is configured', async () => {
+    await expect(GitHubAppService._apiHeaders()).rejects.toThrow('github_not_configured');
+  });
+});

backend/__tests__/unit/services/skillsRefreshService.githubAuth.test.js

@@ -0,0 +1,91 @@
+// skills-refresh GitHub auth: one credential per run, minted through
+// GitHubAppService (App token by default), a failed mint makes no repo request,
+// and with no credential configured the requests go out unauthenticated.
+const fs = require('fs');
+const os = require('os');
+const path = require('path');
+const GitHubAppService = require('../../../services/githubAppService');
+const { refreshSkillsIndex } = require('../../../services/skillsRefreshService');
+
+const jsonResponse = (body) => ({
+  ok: true,
+  status: 200,
+  statusText: 'OK',
+  headers: { get: () => null },
+  text: async () => JSON.stringify(body),
+  json: async () => body,
+});
+
+describe('skillsRefreshService GitHub auth', () => {
+  let tmpDir;
+  let savedPath;
+  let fetchMock;
+
+  beforeEach(() => {
+    tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'skills-refresh-'));
+    savedPath = process.env.SKILLS_CATALOG_PATH;
+    process.env.SKILLS_CATALOG_PATH = path.join(tmpDir, 'index.json');
+    fetchMock = jest.fn(async (url) => {
+      if (String(url).startsWith('https://api.github.com/repos/')) {
+        return jsonResponse({ stargazers_count: 7, forks_count: 1 });
+      }
+      return jsonResponse({ items: [{ repo: 'a/one' }, { repo: 'b/two' }, { repo: 'a/one' }] });
+    });
+    global.fetch = fetchMock;
+    jest.spyOn(console, 'log').mockImplementation(() => {});
+    jest.spyOn(console, 'warn').mockImplementation(() => {});
+  });
+
+  afterEach(() => {
+    if (savedPath === undefined) delete process.env.SKILLS_CATALOG_PATH;
+    else process.env.SKILLS_CATALOG_PATH = savedPath;
+    fs.rmSync(tmpDir, { recursive: true, force: true });
+    delete global.fetch;
+    jest.restoreAllMocks();
+  });
+
+  const repoCalls = () => fetchMock.mock.calls
+    .filter(([url]) => String(url).startsWith('https://api.github.com/repos/'));
+
+  it('mints one App credential per run and sends it on every repo request', async () => {
+    jest.spyOn(GitHubAppService, 'isConfigured').mockReturnValue(true);
+    jest.spyOn(GitHubAppService, 'isPatConfigured').mockReturnValue(true);
+    const headers = jest.spyOn(GitHubAppService, '_apiHeaders')
+      .mockResolvedValue({ Authorization: 'Bearer app-installation-token' });
+
+    const result = await refreshSkillsIndex();
+
+    expect(headers).toHaveBeenCalledTimes(1);
+    expect(repoCalls()).toHaveLength(2);
+    repoCalls().forEach(([, init]) => {
+      expect(init.headers.Authorization).toBe('Bearer app-installation-token');
+    });
+    expect(result.reposUpdated).toBe(2);
+  });
+
+  it('stops before any repo request when the credential mint fails', async () => {
+    jest.spyOn(GitHubAppService, 'isConfigured').mockReturnValue(true);
+    jest.spyOn(GitHubAppService, '_apiHeaders').mockRejectedValue(new Error('mint refused'));
+
+    const result = await refreshSkillsIndex();
+
+    expect(repoCalls()).toHaveLength(0);
+    expect(result.errors.map((e) => e.message)).toContainEqual(expect.stringContaining('mint refused'));
+    expect(result.reposUpdated).toBe(0);
+    expect(result.wroteFile).toBe(true);
+  });
+
+  it('goes unauthenticated when no App or PAT is configured', async () => {
+    jest.spyOn(GitHubAppService, 'isConfigured').mockReturnValue(false);
+    jest.spyOn(GitHubAppService, 'isPatConfigured').mockReturnValue(false);
+    const headers = jest.spyOn(GitHubAppService, '_apiHeaders');
+
+    await refreshSkillsIndex();
+
+    expect(headers).not.toHaveBeenCalled();
+    expect(repoCalls()).toHaveLength(2);
+    repoCalls().forEach(([, init]) => {
+      expect(init.headers.Authorization).toBeUndefined();
+    });
+  });
+});

backend/routes/github.ts

@@ -153,8 +153,10 @@ router.get('/status', auth, async (req: AuthReq, res: Res) => {
   // mapper count; do not add the marker to a networked route.
   try {
     if (req.user?.role !== 'admin') return res.status(403).json({ message: 'Admin only' });
-    if (GitHubAppService.isPatConfigured()) return res.json({ mode: 'pat', configured: true });
-    if (!GitHubAppService.isConfigured()) return res.json({ mode: 'none', configured: false, message: 'Set GITHUB_PAT or GitHub App env vars' });
+    if (!GitHubAppService.isConfigured()) {
+      if (GitHubAppService.isPatConfigured()) return res.json({ mode: 'pat', configured: true });
+      return res.json({ mode: 'none', configured: false, message: 'Set GitHub App env vars' });
+    }
     const appJWT = GitHubAppService.generateAppJWT();
     return res.json({ mode: 'app', configured: true, appId: process.env.GITHUB_APP_ID, installationId: process.env.GITHUB_APP_INSTALLATION_ID_COMMONLY, jwtGenerated: !!appJWT });
   } catch (err) {

backend/services/githubAppService.ts

@@ -176,23 +176,31 @@ class GitHubAppService {
   // ─── Issues API ──────────────────────────────────────────────────────────
 
   /**
-   * Shared headers for GitHub REST API calls (uses PAT or App token).
-   * Supplying an installationId is an app-only request. This keeps a future
-   * broker tool fail-closed even if its caller forgets to pass forceApp.
+   * Shared headers for GitHub REST API calls.
+   *
+   * The App installation token is the default credential: it lives an hour and
+   * is scoped to the repos the App is installed on. The shared GITHUB_PAT is a
+   * fallback for instances with no App configured, and using it is logged.
+   * Supplying an installationId (or forceApp) is an app-only request, so a
+   * broker tool stays fail-closed even if its caller forgets to pass forceApp.
    */
-  static async _apiHeaders(token?: string, installationId?: string, forceApp = false): Promise<Record<string, string>> {
-    const pat = forceApp || installationId ? undefined : (token || process.env.GITHUB_PAT);
-    let credential = pat;
+  static async _apiHeaders(installationId?: string, forceApp = false): Promise<Record<string, string>> {
+    // An explicit installationId is app-only even when it is empty, so a blank
+    // value can neither pick the default installation nor reach the PAT.
+    const appOnly = forceApp || installationId !== undefined;
+    if (installationId !== undefined && !installationId) throw new Error('github_not_configured');
+    const resolvedInstallationId = installationId || process.env.GITHUB_APP_INSTALLATION_ID_COMMONLY;
     const appConfigured = !!(
       process.env.GITHUB_APP_ID
       && process.env.GITHUB_APP_PRIVATE_KEY
-      && (installationId || process.env.GITHUB_APP_INSTALLATION_ID_COMMONLY)
+      && resolvedInstallationId
     );
-    if (!credential && appConfigured) {
-      const installation = await this.getInstallationToken(
-        installationId || process.env.GITHUB_APP_INSTALLATION_ID_COMMONLY as string,
-      );
-      credential = installation.token;
+    let credential: string | undefined;
+    if (appConfigured) {
+      credential = (await this.getInstallationToken(resolvedInstallationId as string)).token;
+    } else if (!appOnly && process.env.GITHUB_PAT) {
+      console.warn('[github] GitHub App not configured; falling back to GITHUB_PAT');
+      credential = process.env.GITHUB_PAT;
     }
     if (!credential) throw new Error('github_not_configured');
     return {
@@ -206,7 +214,7 @@ class GitHubAppService {
    * List open issues for a repo (excludes pull requests).
    */
   static async listOpenIssues({ owner = 'Team-Commonly', repo = 'commonly', perPage = 20, installationId, forceApp }: ListIssuesOptions = {}): Promise<GitHubIssue[]> {
-    const headers = await this._apiHeaders(undefined, installationId, forceApp);
+    const headers = await this._apiHeaders(installationId, forceApp);
     const res = await axios.get(
       `https://api.github.com/repos/${owner}/${repo}/issues?state=open&per_page=${perPage}`,
       { headers },
@@ -218,7 +226,7 @@ class GitHubAppService {
    * Create a new GitHub issue.
    */
   static async createIssue({ owner = 'Team-Commonly', repo = 'commonly', title, body, labels, installationId, forceApp }: CreateIssueOptions): Promise<GitHubIssue> {
-    const headers = await this._apiHeaders(undefined, installationId, forceApp);
+    const headers = await this._apiHeaders(installationId, forceApp);
     const payload: Record<string, unknown> = { title };
     if (body) payload.body = body;
     if (labels?.length) payload.labels = labels;
@@ -234,7 +242,7 @@ class GitHubAppService {
    * Add a comment to an existing issue.
    */
   static async addIssueComment({ owner = 'Team-Commonly', repo = 'commonly', issueNumber, body, installationId, forceApp }: IssueCommentOptions): Promise<unknown> {
-    const headers = await this._apiHeaders(undefined, installationId, forceApp);
+    const headers = await this._apiHeaders(installationId, forceApp);
     const res = await axios.post(
       `https://api.github.com/repos/${owner}/${repo}/issues/${issueNumber}/comments`,
       { body },
@@ -250,7 +258,7 @@ class GitHubAppService {
     if (comment) {
       await this.addIssueComment({ owner, repo, issueNumber, body: comment, installationId, forceApp });
     }
-    const headers = await this._apiHeaders(undefined, installationId, forceApp);
+    const headers = await this._apiHeaders(installationId, forceApp);
     const res = await axios.patch(
       `https://api.github.com/repos/${owner}/${repo}/issues/${issueNumber}`,
       { state: 'closed' },
@@ -261,7 +269,7 @@ class GitHubAppService {
 
   /** Fetch one issue without exposing the server credential to the caller. */
   static async getIssue({ owner = 'Team-Commonly', repo = 'commonly', issueNumber, installationId, forceApp }: IssueNumberOptions): Promise<GitHubIssue> {
-    const headers = await this._apiHeaders(undefined, installationId, forceApp);
+    const headers = await this._apiHeaders(installationId, forceApp);
     const res = await axios.get(
       `https://api.github.com/repos/${owner}/${repo}/issues/${issueNumber}`,
       { headers },
@@ -271,7 +279,7 @@ class GitHubAppService {
 
   /** Fetch one pull request. */
   static async getPullRequest({ owner = 'Team-Commonly', repo = 'commonly', pullNumber, installationId, forceApp }: PullRequestOptions): Promise<unknown> {
-    const headers = await this._apiHeaders(undefined, installationId, forceApp);
+    const headers = await this._apiHeaders(installationId, forceApp);
     const res = await axios.get(
       `https://api.github.com/repos/${owner}/${repo}/pulls/${pullNumber}`,
       { headers },
@@ -281,7 +289,7 @@ class GitHubAppService {
 
   /** List files changed by one pull request. */
   static async listPullRequestFiles({ owner = 'Team-Commonly', repo = 'commonly', pullNumber, installationId, forceApp }: PullRequestOptions): Promise<unknown[]> {
-    const headers = await this._apiHeaders(undefined, installationId, forceApp);
+    const headers = await this._apiHeaders(installationId, forceApp);
     const res = await axios.get(
       `https://api.github.com/repos/${owner}/${repo}/pulls/${pullNumber}/files`,
       { headers },
@@ -294,7 +302,7 @@ class GitHubAppService {
     owner = 'Team-Commonly', repo = 'commonly', pullNumber, mergeMethod, commitTitle, commitMessage,
     installationId, forceApp, sha,
   }: MergePullRequestOptions): Promise<unknown> {
-    const headers = await this._apiHeaders(undefined, installationId, forceApp);
+    const headers = await this._apiHeaders(installationId, forceApp);
     const payload: Record<string, unknown> = {};
     if (sha) payload.sha = sha;
     if (mergeMethod) payload.merge_method = mergeMethod;

backend/services/skillsRefreshService.ts

@@ -1,5 +1,6 @@
 import fs from 'fs';
 import path from 'path';
+import GitHubAppService from './githubAppService';
 
 /**
  * skillsRefreshService
@@ -57,18 +58,19 @@ const resolveLocalCatalogPath = (): string => {
   return path.resolve(__dirname, '../../docs/skills/awesome-agent-skills-index.json');
 };
 
-const buildGitHubHeaders = (): Record<string, string> => {
-  const headers: Record<string, string> = {
+// One credential per refresh run. The App installation token is the default
+// (an hour is longer than a run); GitHubAppService falls back to GITHUB_PAT only
+// when no App is configured. With neither, requests go out unauthenticated at
+// the 60/h anonymous limit, which still covers a small catalog.
+const buildGitHubHeaders = async (): Promise<Record<string, string>> => {
+  const base: Record<string, string> = {
     'User-Agent': 'commonly-skills-refresh',
     Accept: 'application/vnd.github+json',
     'X-GitHub-Api-Version': '2022-11-28',
   };
-  const token = process.env.GITHUB_PAT;
-  if (token) {
-    // Fine-grained PATs use `Bearer`; classic PATs accept it too.
-    headers.Authorization = `Bearer ${token}`;
-  }
-  return headers;
+  if (!GitHubAppService.isConfigured() && !GitHubAppService.isPatConfigured()) return base;
+  const authed = await GitHubAppService._apiHeaders();
+  return { ...base, Authorization: authed.Authorization };
 };
 
 const sleep = (ms: number): Promise<void> => new Promise((resolve) => {
@@ -119,9 +121,10 @@ const parseRepoString = (repo: unknown): { owner: string; name: string } | null
 const fetchRepoStats = async (
   owner: string,
   name: string,
+  headers: Record<string, string>,
 ): Promise<{ stars: number | null; forks: number | null; rateLimited: boolean; authFailed: boolean }> => {
   const url = `${GITHUB_API_BASE}/repos/${encodeURIComponent(owner)}/${encodeURIComponent(name)}`;
-  const response = await fetch(url, { headers: buildGitHubHeaders() });
+  const response = await fetch(url, { headers });
   if (response.status === 401 || response.status === 403) {
     // 403 with x-ratelimit-remaining=0 is the rate-limit signal; distinguish
     // from an auth failure by checking the header.
@@ -213,15 +216,25 @@ export const refreshSkillsIndex = async (): Promise<RefreshResult> => {
   const statsByRepo = new Map<string, { stars: number; forks: number }>();
   let stopDueToAuth = false;
   let stopDueToRateLimit = false;
+  let githubHeaders: Record<string, string> | null = null;
+  if (uniqueRepos.size > 0) {
+    try {
+      githubHeaders = await buildGitHubHeaders();
+    } catch (error) {
+      stopDueToAuth = true;
+      errors.push(new Error(`GitHub App token mint failed: ${(error as Error).message}`));
+    }
+  }
 
   // Walk repos sequentially with a delay — avoids secondary rate-limit bursts.
   for (const [key, parsed] of uniqueRepos) {
+    if (!githubHeaders) break;
     try {
       // eslint-disable-next-line no-await-in-loop
-      const { stars, forks, rateLimited, authFailed } = await fetchRepoStats(parsed.owner, parsed.name);
+      const { stars, forks, rateLimited, authFailed } = await fetchRepoStats(parsed.owner, parsed.name, githubHeaders);
       if (authFailed) {
         stopDueToAuth = true;
-        errors.push(new Error(`GitHub auth failed for ${key} — check GITHUB_PAT`));
+        errors.push(new Error(`GitHub auth failed for ${key} — check the GitHub App credentials`));
         break;
       }
       if (rateLimited) {