diff --git a/package-lock.json b/package-lock.json index ad67c85..6d79474 100644 --- a/package-lock.json +++ b/package-lock.json @@ -9,6 +9,7 @@ "version": "0.1.0", "license": "MIT", "dependencies": { + "@hookform/resolvers": "3.7.0", "@octokit/auth-app": "6.1.1", "@octokit/graphql-schema": "15.18.1", "@primer/octicons-react": "19.9.0", @@ -27,6 +28,7 @@ "proxy-agent": "6.4.0", "react": "18.3.1", "react-dom": "18.3.1", + "react-hook-form": "7.52.1", "simple-git": "3.25.0", "styled-components": "5.3.11", "superjson": "2.2.1", @@ -2042,6 +2044,14 @@ "resolved": "https://registry.npmjs.org/@hapi/bourne/-/bourne-2.1.0.tgz", "integrity": "sha512-i1BpaNDVLJdRBEKeJWkVO6tYX6DMFBuwMhSuWqLsY4ufeTKGVuV5rBsUhxPayXqnnWHgXUAmWK16H/ykO5Wj4Q==" }, + "node_modules/@hookform/resolvers": { + "version": "3.7.0", + "resolved": "https://registry.npmjs.org/@hookform/resolvers/-/resolvers-3.7.0.tgz", + "integrity": "sha512-42p5X18noBV3xqOpTlf2V5qJZwzNgO4eLzHzmKGh/w7z4+4XqRw5AsESVkqE+qwAuRRlg2QG12EVEjPkrRIbeg==", + "peerDependencies": { + "react-hook-form": "^7.0.0" + } + }, "node_modules/@humanwhocodes/config-array": { "version": "0.11.14", "resolved": "https://registry.npmjs.org/@humanwhocodes/config-array/-/config-array-0.11.14.tgz", @@ -12733,6 +12743,21 @@ "react": "^18.3.1" } }, + "node_modules/react-hook-form": { + "version": "7.52.1", + "resolved": "https://registry.npmjs.org/react-hook-form/-/react-hook-form-7.52.1.tgz", + "integrity": "sha512-uNKIhaoICJ5KQALYZ4TOaOLElyM+xipord+Ha3crEFhTntdLvWZqVY49Wqd/0GiVCA/f9NjemLeiNPjG7Hpurg==", + "engines": { + "node": ">=12.22.0" + }, + "funding": { + "type": "opencollective", + "url": "https://opencollective.com/react-hook-form" + }, + "peerDependencies": { + "react": "^16.8.0 || ^17 || ^18 || ^19" + } + }, "node_modules/react-intersection-observer": { "version": "9.5.3", "resolved": "https://registry.npmjs.org/react-intersection-observer/-/react-intersection-observer-9.5.3.tgz", diff --git a/package.json b/package.json index de19a30..44e437b 100644 --- a/package.json +++ b/package.json @@ -19,6 +19,7 @@ "prepare": "test -d node_modules/husky && husky || echo \"husky is not installed\"" }, "dependencies": { + "@hookform/resolvers": "3.7.0", "@octokit/auth-app": "6.1.1", "@octokit/graphql-schema": "15.18.1", "@primer/octicons-react": "19.9.0", @@ -37,6 +38,7 @@ "proxy-agent": "6.4.0", "react": "18.3.1", "react-dom": "18.3.1", + "react-hook-form": "7.52.1", "simple-git": "3.25.0", "styled-components": "5.3.11", "superjson": "2.2.1", diff --git a/src/app/[organizationId]/forks/[forkId]/page.tsx b/src/app/[organizationId]/forks/[forkId]/page.tsx index e58debb..d6dc52d 100644 --- a/src/app/[organizationId]/forks/[forkId]/page.tsx +++ b/src/app/[organizationId]/forks/[forkId]/page.tsx @@ -36,6 +36,7 @@ import Fuse from 'fuse.js' import { useForkData } from 'hooks/useFork' import { useOrgData } from 'hooks/useOrganization' import { useCallback, useState } from 'react' +import { RepositorySettingsSchema } from 'server/repos/schema' const Fork = () => { const { organizationId } = useParams() @@ -204,9 +205,11 @@ const Fork = () => { async ({ repoName, branchName, + settings, }: { repoName: string branchName: string + settings: RepositorySettingsSchema }) => { // close other flashes and dialogs when this is opened closeAllFlashes() @@ -219,6 +222,7 @@ const Fork = () => { forkRepoName: forkData?.data?.name ?? '', forkRepoOwner: forkData?.data?.owner.login ?? '', forkId: String(forkData?.data?.id), + settings, }) .then((res) => { if (res.success) { diff --git a/src/app/components/dialog/CreateMirrorDialog.tsx b/src/app/components/dialog/CreateMirrorDialog.tsx index 9238ba8..537ee31 100644 --- a/src/app/components/dialog/CreateMirrorDialog.tsx +++ b/src/app/components/dialog/CreateMirrorDialog.tsx @@ -1,8 +1,19 @@ -import { Box, FormControl, Label, Link, Text, TextInput } from '@primer/react' +import { zodResolver } from '@hookform/resolvers/zod' +import { + Box, + Checkbox, + FormControl, + Label, + Link, + Select, + Text, + TextInput, +} from '@primer/react' import { Stack } from '@primer/react/lib-esm/Stack' import { Dialog } from '@primer/react/lib-esm/drafts' - import { useState } from 'react' +import { useForm } from 'react-hook-form' +import { RepositorySettingsSchema } from 'server/repos/schema' interface CreateMirrorDialogProps { orgLogin: string @@ -10,7 +21,11 @@ interface CreateMirrorDialogProps { forkParentName: string isOpen: boolean closeDialog: () => void - createMirror: (data: { repoName: string; branchName: string }) => void + createMirror: (data: { + repoName: string + branchName: string + settings: RepositorySettingsSchema + }) => void } export const CreateMirrorDialog = ({ @@ -23,6 +38,9 @@ export const CreateMirrorDialog = ({ }: CreateMirrorDialogProps) => { // set to default value of 'repository-name' for display purposes const [repoName, setRepoName] = useState('repository-name') + const { register, watch, getValues } = useForm({ + resolver: zodResolver(RepositorySettingsSchema), + }) if (!isOpen) { return null @@ -44,7 +62,11 @@ export const CreateMirrorDialog = ({ content: 'Confirm', variant: 'primary', onClick: () => { - createMirror({ repoName, branchName: repoName }) + createMirror({ + repoName, + branchName: repoName, + settings: getValues(), + }) setRepoName('repository-name') }, disabled: repoName === 'repository-name' || repoName === '', @@ -76,7 +98,7 @@ export const CreateMirrorDialog = ({ - + Mirror location + + Mirror Settings + + + + Enable Actions + + + Allowed Actions Settings + + + + ) diff --git a/src/server/repos/controller.ts b/src/server/repos/controller.ts index c9aba8b..67cf253 100644 --- a/src/server/repos/controller.ts +++ b/src/server/repos/controller.ts @@ -2,6 +2,7 @@ /* eslint-disable @typescript-eslint/no-explicit-any */ /* eslint-disable @typescript-eslint/no-unsafe-member-access */ +import { TRPCError } from '@trpc/server' import simpleGit, { SimpleGitOptions } from 'simple-git' import { generateAuthUrl } from 'utils/auth' import { temporaryDirectory } from 'utils/dir' @@ -18,7 +19,6 @@ import { EditMirrorSchema, ListMirrorsSchema, } from './schema' -import { TRPCError } from '@trpc/server' const reposApiLogger = logger.getSubLogger({ name: 'repos-api' }) @@ -136,6 +136,31 @@ export const createMirrorHandler = async ({ }, }) + reposApiLogger.info('Created new repo', { newRepo }) + + // apply settings for actions + await privateOctokit.request( + 'PUT /repos/{owner}/{repo}/actions/permissions', + { + owner: privateOrg, + repo: input.newRepoName, + enabled: input.settings.actions.enabled, + allowed_actions: input.settings.actions.enabled + ? input.settings.actions.allowedActions + : undefined, + headers: { + 'X-GitHub-Api-Version': '2022-11-28', + }, + }, + ) + + reposApiLogger.debug('Applied settings for actions for repo', { + owner: privateOrg, + repo: input.newRepoName, + enabled: input.settings.actions.enabled, + allowed_actions: input.settings.actions.allowedActions, + }) + const defaultBranch = forkData.data.default_branch // Add the mirror remote diff --git a/src/server/repos/schema.ts b/src/server/repos/schema.ts index 676790a..14e26a8 100644 --- a/src/server/repos/schema.ts +++ b/src/server/repos/schema.ts @@ -1,5 +1,12 @@ import { z } from 'zod' +export const RepositorySettingsSchema = z.object({ + actions: z.object({ + enabled: z.boolean(), + allowedActions: z.enum(['all', 'local_only', 'selected']).optional(), + }), +}) + export const CreateMirrorSchema = z.object({ orgId: z.string(), forkRepoOwner: z.string(), @@ -7,6 +14,7 @@ export const CreateMirrorSchema = z.object({ forkId: z.string(), newRepoName: z.string().max(100), newBranchName: z.string(), + settings: RepositorySettingsSchema, }) export const ListMirrorsSchema = z.object({ @@ -29,3 +37,4 @@ export type CreateMirrorSchema = z.TypeOf export type ListMirrorsSchema = z.TypeOf export type EditMirrorSchema = z.TypeOf export type DeleteMirrorSchema = z.TypeOf +export type RepositorySettingsSchema = z.TypeOf diff --git a/test/octomock/index.ts b/test/octomock/index.ts index c61df3a..cc6cc2b 100644 --- a/test/octomock/index.ts +++ b/test/octomock/index.ts @@ -1,6 +1,7 @@ // This is taken from https://github.com/Chocrates/octomock let mockFunctions = { + request: jest.fn(), config: { get: jest.fn(), }, @@ -306,6 +307,7 @@ export class Octomock { mockFunctions: { rest: Record> config: Record + request: jest.Mock } defaultContext: { payload: { issue: { body: string; user: {} } } } diff --git a/test/server/repos.test.ts b/test/server/repos.test.ts index f8e3303..2f081cb 100644 --- a/test/server/repos.test.ts +++ b/test/server/repos.test.ts @@ -14,11 +14,11 @@ jest.mock('simple-git', () => { }) import * as config from '../../src/bot/config' -import * as auth from '../../src/utils/auth' import reposRouter from '../../src/server/repos/router' +import * as auth from '../../src/utils/auth' +import { t } from '../../src/utils/trpc-server' import { Octomock } from '../octomock' import { createTestContext } from '../utils/auth' -import { t } from '../../src/utils/trpc-server' const om = new Octomock() jest.mock('../../src/bot/config') @@ -127,6 +127,9 @@ describe('Repos router', () => { fakeOrgCustomProperties, ) om.mockFunctions.rest.repos.createInOrg.mockResolvedValue(fakeMirrorRepo) + om.mockFunctions.request.mockResolvedValue({ + status: 204, + }) const res = await caller.createMirror({ forkId: 'test', @@ -135,6 +138,11 @@ describe('Repos router', () => { forkRepoOwner: 'github', newBranchName: 'test', newRepoName: 'test', + settings: { + actions: { + enabled: false, + }, + }, }) // TODO: use real git operations and verify fs state after @@ -144,6 +152,19 @@ describe('Repos router', () => { expect(stubbedGit.addRemote).toHaveBeenCalledTimes(1) expect(stubbedGit.push).toHaveBeenCalledTimes(2) expect(stubbedGit.checkoutBranch).toHaveBeenCalledTimes(1) + expect(om.mockFunctions.request).toHaveBeenCalledTimes(1) + expect(om.mockFunctions.request).toHaveBeenCalledWith( + 'PUT /repos/{owner}/{repo}/actions/permissions', + { + owner: 'github-test', + repo: 'test', + enabled: false, + allowed_actions: undefined, + headers: { + 'X-GitHub-Api-Version': '2022-11-28', + }, + }, + ) expect(res).toEqual({ success: true, @@ -165,6 +186,9 @@ describe('Repos router', () => { om.mockFunctions.rest.orgs.get.mockResolvedValue(fakeOrg) om.mockFunctions.rest.repos.get.mockResolvedValue(fakeMirrorRepo) om.mockFunctions.rest.repos.delete.mockResolvedValue({}) + om.mockFunctions.request.mockResolvedValue({ + status: 204, + }) await caller .createMirror({ @@ -174,6 +198,11 @@ describe('Repos router', () => { forkRepoOwner: 'github', newBranchName: 'test', newRepoName: 'test', + settings: { + actions: { + enabled: false, + }, + }, }) .catch((error) => { expect(error.message).toEqual('Repo github-test/test already exists') @@ -183,6 +212,7 @@ describe('Repos router', () => { expect(om.mockFunctions.rest.repos.get).toHaveBeenCalledTimes(1) expect(om.mockFunctions.rest.repos.delete).toHaveBeenCalledTimes(0) expect(stubbedGit.clone).toHaveBeenCalledTimes(0) + expect(om.mockFunctions.request).toHaveBeenCalledTimes(0) }) it('should cleanup repos when there is an error', async () => { @@ -200,6 +230,9 @@ describe('Repos router', () => { om.mockFunctions.rest.repos.get.mockResolvedValueOnce(repoNotFound) om.mockFunctions.rest.repos.get.mockResolvedValueOnce(fakeMirrorRepo) om.mockFunctions.rest.repos.delete.mockResolvedValue({}) + om.mockFunctions.request.mockResolvedValue({ + status: 204, + }) stubbedGit.clone.mockRejectedValue(new Error('clone error')) @@ -211,6 +244,11 @@ describe('Repos router', () => { forkRepoOwner: 'github', newBranchName: 'test', newRepoName: 'test', + settings: { + actions: { + enabled: false, + }, + }, }) .catch((error) => { expect(error.message).toEqual('clone error') @@ -223,6 +261,7 @@ describe('Repos router', () => { repo: 'test', }) expect(stubbedGit.clone).toHaveBeenCalledTimes(1) + expect(om.mockFunctions.request).toHaveBeenCalledTimes(0) }) it('dual-org: should cleanup repos when there is an error', async () => { @@ -251,6 +290,11 @@ describe('Repos router', () => { forkRepoOwner: 'github', newBranchName: 'test', newRepoName: 'test', + settings: { + actions: { + enabled: false, + }, + }, }) .catch((error) => { expect(error.message).toEqual('clone error') @@ -263,6 +307,141 @@ describe('Repos router', () => { repo: 'test', }) expect(stubbedGit.clone).toHaveBeenCalledTimes(1) + expect(om.mockFunctions.request).toHaveBeenCalledTimes(0) + }) + + it('should change settings of a repo for actions', async () => { + const caller = t.createCallerFactory(reposRouter)(createTestContext()) + + const configSpy = jest.spyOn(config, 'getConfig').mockResolvedValue({ + publicOrg: 'github', + privateOrg: 'github-test', + }) + + om.mockFunctions.rest.apps.getOrgInstallation.mockResolvedValue( + fakeOrgInstallation, + ) + om.mockFunctions.rest.orgs.get.mockResolvedValue(fakeOrg) + om.mockFunctions.rest.repos.get.mockResolvedValueOnce(repoNotFound) + om.mockFunctions.rest.repos.get.mockResolvedValueOnce(fakeForkRepo) + om.mockFunctions.rest.orgs.getAllCustomProperties.mockResolvedValue( + fakeOrgCustomProperties, + ) + om.mockFunctions.rest.orgs.createOrUpdateCustomProperty.mockResolvedValue( + fakeOrgCustomProperties, + ) + om.mockFunctions.rest.repos.createInOrg.mockResolvedValue(fakeMirrorRepo) + om.mockFunctions.request.mockResolvedValue({ + status: 204, + }) + + const res = await caller.createMirror({ + forkId: 'test', + orgId: 'test', + forkRepoName: 'fork-test', + forkRepoOwner: 'github', + newBranchName: 'test', + newRepoName: 'test', + settings: { + actions: { + enabled: true, + allowedActions: 'all', + }, + }, + }) + + // TODO: use real git operations and verify fs state after + expect(configSpy).toHaveBeenCalledTimes(1) + expect(om.mockFunctions.rest.repos.get).toHaveBeenCalledTimes(2) + expect(stubbedGit.clone).toHaveBeenCalledTimes(1) + expect(stubbedGit.addRemote).toHaveBeenCalledTimes(1) + expect(stubbedGit.push).toHaveBeenCalledTimes(2) + expect(stubbedGit.checkoutBranch).toHaveBeenCalledTimes(1) + expect(om.mockFunctions.request).toHaveBeenCalledTimes(1) + expect(om.mockFunctions.request).toHaveBeenCalledWith( + 'PUT /repos/{owner}/{repo}/actions/permissions', + { + owner: 'github-test', + repo: 'test', + enabled: true, + allowed_actions: 'all', + headers: { + 'X-GitHub-Api-Version': '2022-11-28', + }, + }, + ) + + expect(res).toEqual({ + success: true, + data: fakeMirrorRepo.data, + }) + }) + + it('allowed_actions should be undefined when actions are not enabled', async () => { + const caller = t.createCallerFactory(reposRouter)(createTestContext()) + + const configSpy = jest.spyOn(config, 'getConfig').mockResolvedValue({ + publicOrg: 'github', + privateOrg: 'github-test', + }) + + om.mockFunctions.rest.apps.getOrgInstallation.mockResolvedValue( + fakeOrgInstallation, + ) + om.mockFunctions.rest.orgs.get.mockResolvedValue(fakeOrg) + om.mockFunctions.rest.repos.get.mockResolvedValueOnce(repoNotFound) + om.mockFunctions.rest.repos.get.mockResolvedValueOnce(fakeForkRepo) + om.mockFunctions.rest.orgs.getAllCustomProperties.mockResolvedValue( + fakeOrgCustomProperties, + ) + om.mockFunctions.rest.orgs.createOrUpdateCustomProperty.mockResolvedValue( + fakeOrgCustomProperties, + ) + om.mockFunctions.rest.repos.createInOrg.mockResolvedValue(fakeMirrorRepo) + om.mockFunctions.request.mockResolvedValue({ + status: 204, + }) + + const res = await caller.createMirror({ + forkId: 'test', + orgId: 'test', + forkRepoName: 'fork-test', + forkRepoOwner: 'github', + newBranchName: 'test', + newRepoName: 'test', + settings: { + actions: { + enabled: false, + allowedActions: 'all', + }, + }, + }) + + // TODO: use real git operations and verify fs state after + expect(configSpy).toHaveBeenCalledTimes(1) + expect(om.mockFunctions.rest.repos.get).toHaveBeenCalledTimes(2) + expect(stubbedGit.clone).toHaveBeenCalledTimes(1) + expect(stubbedGit.addRemote).toHaveBeenCalledTimes(1) + expect(stubbedGit.push).toHaveBeenCalledTimes(2) + expect(stubbedGit.checkoutBranch).toHaveBeenCalledTimes(1) + expect(om.mockFunctions.request).toHaveBeenCalledTimes(1) + expect(om.mockFunctions.request).toHaveBeenCalledWith( + 'PUT /repos/{owner}/{repo}/actions/permissions', + { + owner: 'github-test', + repo: 'test', + enabled: false, + allowed_actions: undefined, + headers: { + 'X-GitHub-Api-Version': '2022-11-28', + }, + }, + ) + + expect(res).toEqual({ + success: true, + data: fakeMirrorRepo.data, + }) }) it('reject repository names over the character limit', async () => { @@ -276,6 +455,11 @@ describe('Repos router', () => { forkRepoOwner: 'github', newBranchName: 'test', newRepoName: 'a'.repeat(101), + settings: { + actions: { + enabled: false, + }, + }, }) .catch((error) => { expect(error.message).toMatch(