fix(security): validate GitHub owner/repo format before gh CLI call

src/modules/githubFeed.js

@@ -16,6 +16,33 @@ import { getConfig } from './config.js';
 
 const execFileAsync = promisify(execFile);
 
+/**
+ * Regex for valid GitHub owner/repo name segments.
+ * Allows alphanumeric characters, dots, hyphens, and underscores.
+ * Prevents path traversal (e.g. `../../users/admin`) via the `gh` CLI.
+ *
+ * @see https://github.com/VolvoxLLC/volvox-bot/issues/160
+ */
+export const VALID_GH_NAME = /^[a-zA-Z0-9][a-zA-Z0-9._-]*$/;
+
+/**
+ * Return true when both owner and repo are safe to pass to the `gh` CLI.
+ *
+ * @param {string} owner
+ * @param {string} repo
+ * @returns {boolean}
+ */
+export function isValidGhRepo(owner, repo) {
+  return (
+    typeof owner === 'string' &&
+    typeof repo === 'string' &&
+    owner.length > 0 &&
+    repo.length > 0 &&
+    VALID_GH_NAME.test(owner) &&
+    VALID_GH_NAME.test(repo)
+  );
+}
+
 /** @type {ReturnType<typeof setInterval> | null} */
 let feedInterval = null;
 
@@ -33,6 +60,10 @@ let pollInFlight = false;
  * @returns {Promise<object[]>} Array of event objects (up to 10)
  */
 export async function fetchRepoEvents(owner, repo) {
+  if (!isValidGhRepo(owner, repo)) {
+    logWarn('GitHub feed: invalid owner/repo format, refusing CLI call', { owner, repo });
+    return [];
+  }
   const { stdout } = await execFileAsync(
     'gh',
     ['api', `repos/${owner}/${repo}/events?per_page=10`],
@@ -256,8 +287,8 @@ async function pollGuildFeed(client, guildId, feedConfig) {
 
   for (const repoFullName of repos) {
     const [owner, repo] = repoFullName.split('/');
-    if (!owner || !repo) {
-      logWarn('GitHub feed: invalid repo format', { guildId, repo: repoFullName });
+    if (!isValidGhRepo(owner, repo)) {
+      logWarn('GitHub feed: invalid owner/repo format, skipping', { guildId, repo: repoFullName });
       continue;
     }
 

tests/modules/githubFeed.test.js

@@ -97,6 +97,7 @@ vi.mock('discord.js', () => {
 
 import { execFile } from 'node:child_process';
 import { getPool } from '../../src/db.js';
+import { warn } from '../../src/logger.js';
 import { getConfig } from '../../src/modules/config.js';
 import {
   buildEmbed,
@@ -105,8 +106,10 @@ import {
   buildPushEmbed,
   buildReleaseEmbed,
   fetchRepoEvents,
+  isValidGhRepo,
   startGithubFeed,
   stopGithubFeed,
+  VALID_GH_NAME,
 } from '../../src/modules/githubFeed.js';
 import { safeSend } from '../../src/utils/safeSend.js';
 
@@ -667,3 +670,133 @@ describe('buildPushEmbed - edge cases', () => {
     expect(embed._data.fields.find((f) => f.name === 'Commits')?.value).toContain('???????');
   });
 });
+
+// ── Security: owner/repo validation (issue #160) ──────────────────────────
+
+describe('isValidGhRepo', () => {
+  it('accepts simple alphanumeric names', () => {
+    expect(isValidGhRepo('VolvoxLLC', 'volvox-bot')).toBe(true);
+  });
+
+  it('accepts names with dots, hyphens, and underscores', () => {
+    expect(isValidGhRepo('my.org', 'repo_name-2')).toBe(true);
+  });
+
+  it('rejects path traversal in owner', () => {
+    expect(isValidGhRepo('../../etc', 'passwd')).toBe(false);
+  });
+
+  it('rejects path traversal in repo', () => {
+    expect(isValidGhRepo('owner', '../../users/admin')).toBe(false);
+  });
+
+  it('rejects slashes in owner', () => {
+    expect(isValidGhRepo('owner/extra', 'repo')).toBe(false);
+  });
+
+  it('rejects slashes in repo', () => {
+    expect(isValidGhRepo('owner', 'repo/extra')).toBe(false);
+  });
+
+  it('rejects empty owner', () => {
+    expect(isValidGhRepo('', 'repo')).toBe(false);
+  });
+
+  it('rejects empty repo', () => {
+    expect(isValidGhRepo('owner', '')).toBe(false);
+  });
+
+  it('rejects non-string owner', () => {
+    expect(isValidGhRepo(null, 'repo')).toBe(false);
+    expect(isValidGhRepo(undefined, 'repo')).toBe(false);
+    expect(isValidGhRepo(42, 'repo')).toBe(false);
+  });
+
+  it('rejects non-string repo', () => {
+    expect(isValidGhRepo('owner', null)).toBe(false);
+  });
+
+  it('rejects spaces in names', () => {
+    expect(isValidGhRepo('owner name', 'repo')).toBe(false);
+    expect(isValidGhRepo('owner', 'repo name')).toBe(false);
+  });
+
+  it('rejects shell metacharacters', () => {
+    expect(isValidGhRepo('owner;id', 'repo')).toBe(false);
+    expect(isValidGhRepo('owner', 'repo&&evil')).toBe(false);
+    expect(isValidGhRepo('owner', 'repo$(evil)')).toBe(false);
+  });
+
+  it('rejects pure-dot owner/repo (path traversal bypass)', () => {
+    expect(isValidGhRepo('.', 'repo')).toBe(false);
+    expect(isValidGhRepo('..', 'repo')).toBe(false);
+    expect(isValidGhRepo('owner', '..')).toBe(false);
+    expect(isValidGhRepo('..', '..')).toBe(false);
+  });
+});
+
+describe('VALID_GH_NAME regex', () => {
+  it('matches valid names', () => {
+    expect(VALID_GH_NAME.test('VolvoxLLC')).toBe(true);
+    expect(VALID_GH_NAME.test('my-repo_v2.0')).toBe(true);
+  });
+
+  it('does not match path traversal', () => {
+    expect(VALID_GH_NAME.test('../etc')).toBe(false);
+    expect(VALID_GH_NAME.test('foo/bar')).toBe(false);
+  });
+
+  it('rejects pure-dot names (path traversal bypass)', () => {
+    expect(VALID_GH_NAME.test('.')).toBe(false);
+    expect(VALID_GH_NAME.test('..')).toBe(false);
+    expect(VALID_GH_NAME.test('...')).toBe(false);
+  });
+});
+
+describe('fetchRepoEvents - validation guard', () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+  });
+
+  it('returns [] and warns for path traversal in owner', async () => {
+    const result = await fetchRepoEvents('../../etc', 'passwd');
+    expect(result).toEqual([]);
+    expect(execFile).not.toHaveBeenCalled();
+    expect(warn).toHaveBeenCalledWith(
+      'GitHub feed: invalid owner/repo format, refusing CLI call',
+      expect.objectContaining({ owner: '../../etc', repo: 'passwd' }),
+    );
+  });
+
+  it('returns [] and warns for path traversal in repo', async () => {
+    const result = await fetchRepoEvents('owner', '../../users/admin');
+    expect(result).toEqual([]);
+    expect(execFile).not.toHaveBeenCalled();
+  });
+
+  it('returns [] for empty owner', async () => {
+    const result = await fetchRepoEvents('', 'repo');
+    expect(result).toEqual([]);
+    expect(execFile).not.toHaveBeenCalled();
+  });
+
+  it('returns [] for empty repo', async () => {
+    const result = await fetchRepoEvents('owner', '');
+    expect(result).toEqual([]);
+    expect(execFile).not.toHaveBeenCalled();
+  });
+
+  it('proceeds with gh CLI for valid owner/repo', async () => {
+    execFile.mockImplementation((_cmd, _args, _opts, cb) => {
+      cb(null, { stdout: JSON.stringify([{ id: '1', type: 'PushEvent' }]) });
+    });
+    const result = await fetchRepoEvents('VolvoxLLC', 'volvox-bot');
+    expect(result).toHaveLength(1);
+    expect(execFile).toHaveBeenCalledWith(
+      'gh',
+      ['api', 'repos/VolvoxLLC/volvox-bot/events?per_page=10'],
+      { timeout: 30_000 },
+      expect.any(Function),
+    );
+  });
+});