From 47250a4ac8356a01648650a26a748902c7ef5f6b Mon Sep 17 00:00:00 2001 From: Laura Yu <60276246+lauraway@users.noreply.github.com> Date: Tue, 7 Mar 2023 13:21:59 -0800 Subject: [PATCH] Check with repo-permission before accessing secrets and variables (#175) --- .../src/context-providers/secrets.ts | 95 ++++++++++--------- .../src/context-providers/variables.ts | 95 ++++++++++--------- languageserver/src/utils/repo-permission.ts | 46 +++++++++ languageserver/src/utils/username.ts | 16 ++++ 4 files changed, 160 insertions(+), 92 deletions(-) create mode 100644 languageserver/src/utils/repo-permission.ts create mode 100644 languageserver/src/utils/username.ts diff --git a/languageserver/src/context-providers/secrets.ts b/languageserver/src/context-providers/secrets.ts index 217b417..c5cf4b3 100644 --- a/languageserver/src/context-providers/secrets.ts +++ b/languageserver/src/context-providers/secrets.ts @@ -5,6 +5,7 @@ import {isMapping, isString} from "@github/actions-workflow-parser"; import {Octokit} from "@octokit/rest"; import {RepositoryContext} from "../initializationOptions"; import {TTLCache} from "../utils/cache"; +import {getRepoPermission} from "../utils/repo-permission"; export async function getSecrets( workflowContext: WorkflowContext, @@ -13,6 +14,13 @@ export async function getSecrets( repo: RepositoryContext, defaultContext: DescriptionDictionary | undefined ): Promise { + const permission = await getRepoPermission(octokit, cache, repo); + if (permission === "none") { + const secretsContext = defaultContext || new DescriptionDictionary(); + secretsContext.complete = false; + return secretsContext; + } + let environmentName: string | undefined; if (workflowContext?.job?.environment) { if (isString(workflowContext.job.environment)) { @@ -30,54 +38,49 @@ export async function getSecrets( } const secretsContext = defaultContext || new DescriptionDictionary(); - try { - const secrets = await getRemoteSecrets(octokit, cache, repo, environmentName); + const secrets = await getRemoteSecrets(octokit, cache, repo, environmentName); - // Build combined map of secrets - const secretsMap = new Map< - string, - { - key: string; - value: data.StringData; - description?: string; - } - >(); - - secrets.orgSecrets.forEach(secret => - secretsMap.set(secret.value.toLowerCase(), { - key: secret.value, - value: new data.StringData("***"), - description: "Organization secret" - }) - ); - - // Override org secrets with repo secrets - secrets.repoSecrets.forEach(secret => - secretsMap.set(secret.value.toLowerCase(), { - key: secret.value, - value: new data.StringData("***"), - description: "Repository secret" - }) - ); - - // Override repo secrets with environment secrets (if defined) - secrets.environmentSecrets.forEach(secret => - secretsMap.set(secret.value.toLowerCase(), { - key: secret.value, - value: new data.StringData("***"), - description: `Secret for environment \`${environmentName}\`` - }) - ); - - // Sort secrets by key and add to context - Array.from(secretsMap.values()) - .sort((a, b) => a.key.localeCompare(b.key)) - .forEach(secret => secretsContext?.add(secret.key, secret.value, secret.description)); - } catch (e: any) { - if (e.status === 403 || e.status === 404) { - secretsContext.complete = false; + // Build combined map of secrets + const secretsMap = new Map< + string, + { + key: string; + value: data.StringData; + description?: string; } - } + >(); + + secrets.orgSecrets.forEach(secret => + secretsMap.set(secret.value.toLowerCase(), { + key: secret.value, + value: new data.StringData("***"), + description: "Organization secret" + }) + ); + + // Override org secrets with repo secrets + secrets.repoSecrets.forEach(secret => + secretsMap.set(secret.value.toLowerCase(), { + key: secret.value, + value: new data.StringData("***"), + description: "Repository secret" + }) + ); + + // Override repo secrets with environment secrets (if defined) + secrets.environmentSecrets.forEach(secret => + secretsMap.set(secret.value.toLowerCase(), { + key: secret.value, + value: new data.StringData("***"), + description: `Secret for environment \`${environmentName}\`` + }) + ); + + // Sort secrets by key and add to context + Array.from(secretsMap.values()) + .sort((a, b) => a.key.localeCompare(b.key)) + .forEach(secret => secretsContext?.add(secret.key, secret.value, secret.description)); + return secretsContext; } diff --git a/languageserver/src/context-providers/variables.ts b/languageserver/src/context-providers/variables.ts index f78c650..aa3121b 100644 --- a/languageserver/src/context-providers/variables.ts +++ b/languageserver/src/context-providers/variables.ts @@ -6,6 +6,7 @@ import {Pair} from "@github/actions-expressions/data/expressiondata"; import {RepositoryContext} from "../initializationOptions"; import {StringData} from "@github/actions-expressions/data/index"; import {TTLCache} from "../utils/cache"; +import {getRepoPermission} from "../utils/repo-permission"; export async function getVariables( workflowContext: WorkflowContext, @@ -14,6 +15,13 @@ export async function getVariables( repo: RepositoryContext, defaultContext: DescriptionDictionary | undefined ): Promise { + const permission = await getRepoPermission(octokit, cache, repo); + if (permission === "none") { + const secretsContext = defaultContext || new DescriptionDictionary(); + secretsContext.complete = false; + return secretsContext; + } + let environmentName: string | undefined; if (workflowContext?.job?.environment) { if (isString(workflowContext.job.environment)) { @@ -31,54 +39,49 @@ export async function getVariables( } const variablesContext = defaultContext || new DescriptionDictionary(); - try { - const variables = await getRemoteVariables(octokit, cache, repo, environmentName); + const variables = await getRemoteVariables(octokit, cache, repo, environmentName); - // Build combined map of variables - const variablesMap = new Map< - string, - { - key: string; - value: data.StringData; - description?: string; - } - >(); - - variables.organizationVariables.forEach(variable => - variablesMap.set(variable.key.toLowerCase(), { - key: variable.key, - value: new data.StringData(variable.value.coerceString()), - description: `${variable.value.coerceString()} - Organization variable` - }) - ); - - // Override org variables with repo variables - variables.repoVariables.forEach(variable => - variablesMap.set(variable.key.toLowerCase(), { - key: variable.key, - value: new data.StringData(variable.value.coerceString()), - description: `${variable.value.coerceString()} - Repository variable` - }) - ); - - // Override repo variables with environment veriables (if defined) - variables.environmentVariables.forEach(variable => - variablesMap.set(variable.key.toLowerCase(), { - key: variable.key, - value: new data.StringData(variable.value.coerceString()), - description: `${variable.value.coerceString()} - Variable for environment \`${environmentName}\`` - }) - ); - - // Sort variables by key and add to context - Array.from(variablesMap.values()) - .sort((a, b) => a.key.localeCompare(b.key)) - .forEach(variable => variablesContext?.add(variable.key, variable.value, variable.description)); - } catch (e: any) { - if (e.status === 403 || e.status === 404) { - variablesContext.complete = false; + // Build combined map of variables + const variablesMap = new Map< + string, + { + key: string; + value: data.StringData; + description?: string; } - } + >(); + + variables.organizationVariables.forEach(variable => + variablesMap.set(variable.key.toLowerCase(), { + key: variable.key, + value: new data.StringData(variable.value.coerceString()), + description: `${variable.value.coerceString()} - Organization variable` + }) + ); + + // Override org variables with repo variables + variables.repoVariables.forEach(variable => + variablesMap.set(variable.key.toLowerCase(), { + key: variable.key, + value: new data.StringData(variable.value.coerceString()), + description: `${variable.value.coerceString()} - Repository variable` + }) + ); + + // Override repo variables with environment veriables (if defined) + variables.environmentVariables.forEach(variable => + variablesMap.set(variable.key.toLowerCase(), { + key: variable.key, + value: new data.StringData(variable.value.coerceString()), + description: `${variable.value.coerceString()} - Variable for environment \`${environmentName}\`` + }) + ); + + // Sort variables by key and add to context + Array.from(variablesMap.values()) + .sort((a, b) => a.key.localeCompare(b.key)) + .forEach(variable => variablesContext?.add(variable.key, variable.value, variable.description)); + return variablesContext; } diff --git a/languageserver/src/utils/repo-permission.ts b/languageserver/src/utils/repo-permission.ts new file mode 100644 index 0000000..5cb760d --- /dev/null +++ b/languageserver/src/utils/repo-permission.ts @@ -0,0 +1,46 @@ +import {error} from "@github/actions-languageservice/log"; +import {Octokit} from "@octokit/rest"; +import {RepositoryContext} from "../initializationOptions"; +import {TTLCache} from "./cache"; +import {getUsername} from "./username"; + +export type RepoPermission = "admin" | "write" | "read" | "none"; + +export async function getRepoPermission( + octokit: Octokit, + cache: TTLCache, + repo: RepositoryContext +): Promise { + const username = await getUsername(octokit, cache); + const permission = await cache.get(`${repo.owner}/${repo.name}/${username}/permission`, undefined, () => + fetchRepoPermission(octokit, repo, username) + ); + + switch (permission) { + case "admin": + case "write": + case "read": + case "none": + return permission; + default: + error(`Unknown permission: ${permission}`); + return "none"; + } +} + +async function fetchRepoPermission(octokit: Octokit, repo: RepositoryContext, username: string): Promise { + try { + const res = await octokit.request("GET /repos/{owner}/{repo}/collaborators/{username}/permission", { + owner: repo.owner, + repo: repo.name, + username: username + }); + const permission = res.data?.permission; + return permission; + } catch (e: any) { + if (e.status === 404 || e.status === 403) { + return "none"; + } + throw e; + } +} diff --git a/languageserver/src/utils/username.ts b/languageserver/src/utils/username.ts new file mode 100644 index 0000000..e09f79d --- /dev/null +++ b/languageserver/src/utils/username.ts @@ -0,0 +1,16 @@ +import {Octokit} from "@octokit/rest"; +import {TTLCache} from "./cache"; + +export async function getUsername(octokit: Octokit, cache: TTLCache): Promise { + return await cache.get(`/username`, undefined, () => fetchUsername(octokit)); +} + +async function fetchUsername(octokit: Octokit): Promise { + try { + const username = await octokit.request("GET /user").then(res => res.data.login); + return username; + } catch (e) { + console.log("Failure to retrieve username: ", e); + throw e; + } +}