Restrict DevFlow PR reviews to internal contributors

.github/actions/github-app-token/action.yml

@@ -31,7 +31,7 @@ inputs:
     description: Repository to include in the installation token
     required: false
   permission-profile:
-    description: Permission profile (issues, pull-requests, or devflow)
+    description: Permission profile (issues, pull-requests, devflow, or team-check)
     required: true
   fallback-token:
     description: PAT used temporarily when app authentication is unavailable

.github/actions/github-app-token/create-token.js

@@ -16,6 +16,10 @@ const PERMISSION_PROFILES = Object.freeze({
     issues: 'write',
     pull_requests: 'write',
   }),
+  'team-check': Object.freeze({
+    members: 'read',
+    pull_requests: 'read',
+  }),
 });
 
 function base64Url(value) {
@@ -71,7 +75,7 @@ async function createInstallationToken(config, dependencies = {}) {
     throw new Error('TARGET_REPOSITORY must use the owner/repository format.');
   }
   if (!Object.hasOwn(PERMISSION_PROFILES, config.permissionProfile)) {
-    throw new Error('PERMISSION_PROFILE must be issues, pull-requests, or devflow.');
+    throw new Error('PERMISSION_PROFILE must be issues, pull-requests, devflow, or team-check.');
   }
   const permissions = PERMISSION_PROFILES[config.permissionProfile];
 

.github/scripts/check_team_membership.js

@@ -0,0 +1,46 @@
+// Copyright (c) Microsoft. All rights reserved.
+
+async function checkTeamMembership({ github, context, teamSlug, prNumber, username = '' }) {
+  if (!teamSlug) {
+    throw new Error('DEVELOPER_TEAM must be configured with the developer team slug.');
+  }
+
+  const number = Number(prNumber);
+  if (!/^[1-9][0-9]*$/.test(prNumber) || !Number.isSafeInteger(number)) {
+    throw new Error('A valid PR number is required to check team membership.');
+  }
+
+  let author = username.trim() || context.payload?.pull_request?.user?.login;
+  if (!author) {
+    const { data: pr } = await github.rest.pulls.get({
+      ...context.repo,
+      pull_number: number,
+    });
+    author = pr.user?.login;
+  }
+  if (!author) {
+    throw new Error('Could not determine PR author (user may be deleted).');
+  }
+
+  // Verify the team exists so a missing or inaccessible team is not mistaken for a nonmember.
+  await github.rest.teams.getByName({
+    org: context.repo.owner,
+    team_slug: teamSlug,
+  });
+
+  try {
+    const { data: membership } = await github.rest.teams.getMembershipForUserInOrg({
+      org: context.repo.owner,
+      team_slug: teamSlug,
+      username: author,
+    });
+    return { author, isTeamMember: membership.state === 'active' };
+  } catch (error) {
+    if (error.status === 404) {
+      return { author, isTeamMember: false };
+    }
+    throw error;
+  }
+}
+
+module.exports = checkTeamMembership;

.github/tests/test_check_team_membership.js

@@ -0,0 +1,117 @@
+// Copyright (c) Microsoft. All rights reserved.
+
+const { describe, it } = require('node:test');
+const assert = require('node:assert/strict');
+
+const checkTeamMembership = require('../scripts/check_team_membership.js');
+
+const context = { repo: { owner: 'microsoft', repo: 'semantic-kernel' } };
+
+function githubWithMembership(membership, username = 'contributor') {
+  return {
+    rest: {
+      pulls: {
+        get: async (params) => {
+          assert.deepEqual(params, { ...context.repo, pull_number: 42 });
+          return { data: { user: { login: 'contributor' } } };
+        },
+      },
+      teams: {
+        getByName: async (params) => {
+          assert.deepEqual(params, { org: 'microsoft', team_slug: 'developers' });
+        },
+        getMembershipForUserInOrg: async (params) => {
+          assert.deepEqual(params, {
+            org: 'microsoft',
+            team_slug: 'developers',
+            username,
+          });
+          if (membership instanceof Error) throw membership;
+          return { data: { state: membership } };
+        },
+      },
+    },
+  };
+}
+
+describe('Review requester team membership', () => {
+  const options = { context, teamSlug: 'developers', prNumber: '42' };
+
+  it('uses the PR author from the event on automatic runs', async () => {
+    const github = githubWithMembership('active');
+    github.rest.pulls.get = async () => {
+      throw new Error('Should not look up the PR when the event includes its author');
+    };
+    assert.deepEqual(
+      await checkTeamMembership({
+        ...options,
+        context: { ...context, payload: { pull_request: { user: { login: 'contributor' } } } },
+        github,
+      }),
+      { author: 'contributor', isTeamMember: true },
+    );
+  });
+
+  it('uses the initiator on manual runs, even if the PR author is external', async () => {
+    const github = githubWithMembership('active', 'maintainer');
+    github.rest.pulls.get = async () => {
+      throw new Error('Should not look up the PR author on manual runs');
+    };
+    assert.deepEqual(
+      await checkTeamMembership({ ...options, github, username: 'maintainer' }),
+      { author: 'maintainer', isTeamMember: true },
+    );
+    assert.deepEqual(
+      await checkTeamMembership({
+        ...options,
+        github: githubWithMembership(Object.assign(new Error('Not Found'), { status: 404 }), 'outsider'),
+        username: 'outsider',
+      }),
+      { author: 'outsider', isTeamMember: false },
+    );
+  });
+
+  it('accepts an active team member', async () => {
+    assert.deepEqual(
+      await checkTeamMembership({ ...options, github: githubWithMembership('active') }),
+      { author: 'contributor', isTeamMember: true },
+    );
+  });
+
+  it('rejects a pending team member and an outsider', async () => {
+    for (const membership of ['pending', Object.assign(new Error('Not Found'), { status: 404 })]) {
+      assert.deepEqual(
+        await checkTeamMembership({ ...options, github: githubWithMembership(membership) }),
+        { author: 'contributor', isTeamMember: false },
+      );
+    }
+  });
+
+  it('does not treat API errors or a missing team as nonmembership', async () => {
+    await assert.rejects(
+      checkTeamMembership({
+        ...options,
+        github: githubWithMembership(Object.assign(new Error('Forbidden'), { status: 403 })),
+      }),
+      /Forbidden/,
+    );
+    const github = githubWithMembership('active');
+    github.rest.teams.getByName = async () => {
+      throw Object.assign(new Error('Team not found'), { status: 404 });
+    };
+    await assert.rejects(checkTeamMembership({ ...options, github }), /Team not found/);
+  });
+
+  it('rejects missing configuration and invalid PR numbers', async () => {
+    await assert.rejects(
+      checkTeamMembership({ ...options, github: githubWithMembership('active'), teamSlug: '' }),
+      /DEVELOPER_TEAM/,
+    );
+    for (const prNumber of ['0', '1e2', '9007199254740992']) {
+      await assert.rejects(
+        checkTeamMembership({ ...options, github: githubWithMembership('active'), prNumber }),
+        /valid PR number/,
+      );
+    }
+  });
+});

.github/tests/test_create_github_app_token.js

@@ -109,6 +109,28 @@ describe('GitHub App token creation', () => {
     });
   });
 
+  it('limits team-check tokens to member and pull-request reads', async () => {
+    let request;
+    await createInstallationToken(
+      { ...CONFIG, permissionProfile: 'team-check' },
+      {
+        execute: () => '+/8=\n',
+        fetch: async (_url, options) => {
+          request = options;
+          return {
+            ok: true,
+            json: async () => ({ token: 'installation-token' }),
+          };
+        },
+      },
+    );
+
+    assert.deepEqual(JSON.parse(request.body).permissions, {
+      members: 'read',
+      pull_requests: 'read',
+    });
+  });
+
   it('rejects unknown permission profiles before signing', async () => {
     let signed = false;
 
@@ -122,7 +144,7 @@ describe('GitHub App token creation', () => {
           },
         },
       ),
-      /PERMISSION_PROFILE must be issues, pull-requests, or devflow/,
+      /PERMISSION_PROFILE must be issues, pull-requests, devflow, or team-check/,
     );
     assert.equal(signed, false);
   });
@@ -140,7 +162,7 @@ describe('GitHub App token creation', () => {
           },
         },
       ),
-      /PERMISSION_PROFILE must be issues, pull-requests, or devflow/,
+      /PERMISSION_PROFILE must be issues, pull-requests, devflow, or team-check/,
     );
     assert.equal(signed, false);
   });