From 721f671fc8b01687f75c768c9a6b5d913b6dae0e Mon Sep 17 00:00:00 2001 From: Ralf Anton Beier Date: Fri, 1 May 2026 19:23:07 +0200 Subject: [PATCH] fix: gate issues.opened provisioning behind allowed_command_users + org membership MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Why Wave-1 Security auditor flagged this as Bug #4 in `docs/agent-fleet/bugs.md`: comment-driven ChatOps already required `requireOrgMember` AND `allowed_command_users`, but the parallel issue-form provisioning path (`controller_repo`, Cut A) had no such gate. Anyone able to file an issue with the configured `repo-request` label could trigger real org-repo creation, including via the public GitHub API. ## What Two-layer gate inserted *before* validation/enqueue in the `issues.opened` handler's controller_repo branch: 1. **`allowed_command_users`** — same allowlist that comment-driven ChatOps consults. Empty list = open to everyone (back-compat). 2. **`checkOrganizationMembership`** — defence in depth. The sender must be a member of the controller repo's owner org. The function already returns `false` on 404 / network errors, so an uncertain-membership case fails closed. Each rejection posts a specific, actionable error comment back on the issue. Sender is now read out of `context.payload.sender`. ## Source Bug #4, wave-1 Security auditor (`docs/agent-fleet/bugs.md`). ## Test plan - [x] 836 tests pass (was 834; +2 covering: not-allowed-user rejected with `not authorised` comment; sender failing org-membership check rejected with `must be a member` comment) - [x] eslint clean - [ ] After deploy: open a `repo-request`-labelled issue from a non-allowed-list account in `pulseengine/repo-requests` (when `controller_repo.enabled: true`). Bot should refuse rather than provision. ## Risk & rollout - Risk: low. Default behaviour for `allowed_command_users: []` is unchanged (still requires org membership, which existing maintainers already have). Anyone who'd previously rely on "no allowlist + no org check" must now be in the org — which is a sensible default for issue-form provisioning. - Rollout: self-update on merge. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.7 (1M context) --- __tests__/integration/app.test.js | 36 +++++++++++++++++++++++++++++++ src/app.js | 34 ++++++++++++++++++++++++++--- 2 files changed, 67 insertions(+), 3 deletions(-) diff --git a/__tests__/integration/app.test.js b/__tests__/integration/app.test.js index da1cb5a..5b0de65 100644 --- a/__tests__/integration/app.test.js +++ b/__tests__/integration/app.test.js @@ -351,11 +351,18 @@ describe('app', () => { owner: { login: 'pulseengine' }, ...(overrides.repository || {}) }, + sender: { login: 'avrabe', ...(overrides.sender || {}) }, ...(overrides.payload || {}) } }; } + beforeEach(() => { + // Bug #4 added an org-membership gate before provisioning. + // Tests below assume the sender IS a member (true). + checkOrganizationMembership.mockResolvedValue(true); + }); + it('ignores issues when controller_repo is disabled', async () => { _setConfigForTesting({}); const { handlers } = setupApp(); @@ -409,6 +416,35 @@ describe('app', () => { const body = context.octokit.issues.createComment.mock.calls[0][0].body; expect(body).toMatch(/task store offline/i); }); + + // Bug #4 — auth gate before provisioning + it('rejects sender not in allowed_command_users with a comment', async () => { + _setConfigForTesting({ + allowed_command_users: ['avrabe'], + controller_repo: { enabled: true, repo: 'pulseengine/repo-requests', label: 'repo-request' } + }); + const { handlers } = setupApp(); + const context = createIssueOpenedContext({ + sender: { login: 'random-stranger' } + }); + await handlers['issues.opened'](context); + const body = context.octokit.issues.createComment.mock.calls[0][0].body; + expect(body).toMatch(/not authorised/i); + // Critically: did NOT proceed to enqueue / post "Request accepted" + expect(body).not.toMatch(/Request accepted/i); + }); + + it('rejects sender who fails the org-membership check', async () => { + _setConfigForTesting({ + controller_repo: { enabled: true, repo: 'pulseengine/repo-requests', label: 'repo-request' } + }); + checkOrganizationMembership.mockResolvedValue(false); + const { handlers } = setupApp(); + const context = createIssueOpenedContext(); + await handlers['issues.opened'](context); + const body = context.octokit.issues.createComment.mock.calls[0][0].body; + expect(body).toMatch(/must be a member/i); + }); }); // ========================================================================= diff --git a/src/app.js b/src/app.js index db1446d..ee13376 100644 --- a/src/app.js +++ b/src/app.js @@ -365,11 +365,39 @@ function registerApp(app, options = {}) { return; } - const fields = parseIssueFormBody(issue.body || ''); - const validation = validateProvisionRequest(fields); - const owner = repository.owner.login; const repo = repository.name; + const senderLogin = sender?.login; + + // Auth gate: provisioning a real org repo is a high-trust action. + // Comment-driven ChatOps already requires `allowed_command_users` AND + // org membership; the issue-form path (Cut A provisioning) had no such + // gate before this commit (Bug #4 in `docs/agent-fleet/bugs.md`). + // Anyone able to file a labelled issue could trigger repo creation. + const config = getConfig(); + const allowedUsers = config?.allowed_command_users || []; + if (allowedUsers.length > 0 && !allowedUsers.includes(senderLogin)) { + await issueOps.createComment(context.octokit, { + owner, repo, issue_number: issue.number, + body: `❌ You are not authorised to provision repos. Allowed users: ${allowedUsers.join(', ')}.` + }); + if (deliveryId) markProcessed(deliveryId); + return; + } + // Defence in depth: also require org membership in the controller repo's + // owner org. checkOrganizationMembership returns false on 404 / network + // errors — if it can't confirm membership, refuse. + if (!(await checkOrganizationMembership(context.octokit, owner, senderLogin))) { + await issueOps.createComment(context.octokit, { + owner, repo, issue_number: issue.number, + body: '❌ You must be a member of the organisation to provision repos.' + }); + if (deliveryId) markProcessed(deliveryId); + return; + } + + const fields = parseIssueFormBody(issue.body || ''); + const validation = validateProvisionRequest(fields); if (!validation.valid) { await issueOps.createComment(context.octokit,{