Skip to content

Commit

Permalink
feat: verify OAuth scopes of classic GitHub PATs (#897)
Browse files Browse the repository at this point in the history 
* fix: verify OAuth scopes of classic GitHub PATs

* fix: make EGHNOPERMISSION error message clearer

* chore: add comment about x-oauth-scopes header

* test: fix failing test

* test: add integration test for no maintain permission

---------

Co-authored-by: Olabode Lawal-Shittabey <babblebey@gmail.com>
  • Loading branch information
@jedwards1211 @babblebey
jedwards1211 and babblebey authored Sep 2, 2024
1 parent 8061687 commit be071a2
Show file tree
Hide file tree
Showing 4 changed files with 420 additions and 19 deletions.
18 changes: 15 additions & 3 deletions lib/definitions/errors.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -178,12 +178,24 @@ If you are using [GitHub Enterprise](https://enterprise.github.com) please make

export function EGHNOPERMISSION({ owner, repo }) {
return {
message: `The GitHub token doesn't allow to push on the repository ${owner}/${repo}.`,
message: `The GitHub token doesn't allow to push to and maintain the repository ${owner}/${repo}.`,
details: `The user associated with the [GitHub token](${linkify(
"README.md#github-authentication",
)}) configured in the \`GH_TOKEN\` or \`GITHUB_TOKEN\` environment variable must allows to push to the repository ${owner}/${repo}.
)}) configured in the \`GH_TOKEN\` or \`GITHUB_TOKEN\` environment variable must have permission to push to and maintain the repository ${owner}/${repo}.
Please make sure the GitHub user associated with the token is an [owner](https://help.github.com/articles/permission-levels-for-a-user-account-repository/#owner-access-on-a-repository-owned-by-a-user-account) or a [collaborator](https://help.github.com/articles/permission-levels-for-a-user-account-repository/#collaborator-access-on-a-repository-owned-by-a-user-account) if the repository belong to a user account or has [write permissions](https://help.github.com/articles/managing-team-access-to-an-organization-repository) if the repository [belongs to an organization](https://help.github.com/articles/repository-permission-levels-for-an-organization).`,
Please make sure the GitHub user associated with the token is an [owner](https://help.github.com/articles/permission-levels-for-a-user-account-repository/#owner-access-on-a-repository-owned-by-a-user-account) or a [collaborator](https://help.github.com/articles/permission-levels-for-a-user-account-repository/#collaborator-access-on-a-repository-owned-by-a-user-account) if the repository belongs to a user account or has [write permissions](https://help.github.com/articles/managing-team-access-to-an-organization-repository) if the repository [belongs to an organization](https://help.github.com/articles/repository-permission-levels-for-an-organization).`,
};
}

export function EGHNOSCOPE({ scopes }) {
return {
message: `The GitHub token doesn't have the necessary OAuth scopes to write contents, issues, and pull requests.`,
details: `The [GitHub token](${linkify(
"README.md#github-authentication",
)}) configured in the \`GH_TOKEN\` or \`GITHUB_TOKEN\` environment variable must have the correct scopes.
${scopes ? `\nThe token you used has scopes: ${scopes.join(", ")}\n` : ""}
For classic PATs, make sure the token has the \`repo\` scope if the repository is private, or \`public_repo\` scope otherwise.
For fine-grained PATs, make sure the token has the \`content: write\`, \`issues: write\`, and \`pull_requests: write\` scopes on the repository.`,
};
}

Expand Down
17 changes: 15 additions & 2 deletions lib/verify.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,21 @@ export default async function verify(pluginConfig, context, { Octokit }) {
);
try {
const {
data: { permissions, clone_url },
headers,
data: { private: _private, permissions, clone_url },
} = await octokit.request("GET /repos/{owner}/{repo}", { repo, owner });

// GitHub only returns this header if the token is a classic PAT
if (headers?.["x-oauth-scopes"]) {
const scopes = headers["x-oauth-scopes"].split(/\s*,\s*/g);
if (
!scopes.includes("repo") &&
(_private || !scopes.includes("public_repo"))
) {
errors.push(getError("EGHNOSCOPE", { scopes }));
}
}

// Verify if Repository Name wasn't changed
const parsedCloneUrl = parseGithubUrl(clone_url);
if (
Expand All @@ -122,7 +135,7 @@ export default async function verify(pluginConfig, context, { Octokit }) {
// Do not check for permissions in GitHub actions, as the provided token is an installation access token.
// octokit.request("GET /repos/{owner}/{repo}", {repo, owner}) does not return the "permissions" key in that case.
// But GitHub Actions have all permissions required for @semantic-release/github to work
if (!env.GITHUB_ACTION && !permissions?.push) {
if (!env.GITHUB_ACTION && !(permissions?.push && permissions?.maintain)) {
// If authenticated as GitHub App installation, `push` will always be false.
// We send another request to check if current authentication is an installation.
// Note: we cannot check if the installation has all required permissions, it's
Expand Down
68 changes: 63 additions & 5 deletions test/integration.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ test("Verify GitHub auth", async (t) => {
.getOnce(`https://api.github.local/repos/${owner}/${repo}`, {
permissions: {
push: true,
maintain: true,
},
clone_url: `https://api.github.local/${owner}/${repo}.git`,
});
Expand All @@ -49,6 +50,43 @@ test("Verify GitHub auth", async (t) => {
t.true(fetch.done());
});

test("Throws when GitHub user lacks maintain permission", async (t) => {
const owner = "test_user";
const repo = "test_repo";
const env = { GITHUB_TOKEN: "github_token" };
const options = {
repositoryUrl: `git+https://othertesturl.com/${owner}/${repo}.git`,
};

const fetch = fetchMock
.sandbox()
.getOnce(`https://api.github.local/repos/${owner}/${repo}`, {
permissions: {
push: true,
maintain: false,
},
clone_url: `https://api.github.local/${owner}/${repo}.git`,
});

const {
errors: [error],
} = await t.throwsAsync(
t.context.m.verifyConditions(
{},
{ cwd, env, options, logger: t.context.logger },
{
Octokit: TestOctokit.defaults((options) => ({
...options,
request: { ...options.request, fetch },
})),
},
),
);

t.is(error.code, "EGHNOPERMISSION");
t.true(fetch.done());
});

test("Verify GitHub auth with publish options", async (t) => {
const owner = "test_user";
const repo = "test_repo";
Expand All @@ -62,6 +100,7 @@ test("Verify GitHub auth with publish options", async (t) => {
.get(`https://api.github.local/repos/${owner}/${repo}`, {
permissions: {
push: true,
maintain: true,
},
clone_url: `https://api.github.local/${owner}/${repo}.git`,
});
Expand Down Expand Up @@ -102,6 +141,7 @@ test("Verify GitHub auth and assets config", async (t) => {
.getOnce(`https://api.github.local/repos/${owner}/${repo}`, {
permissions: {
push: true,
maintain: true,
},
clone_url: `https://api.github.local/${owner}/${repo}.git`,
});
Expand Down Expand Up @@ -208,6 +248,7 @@ test("Publish a release with an array of assets", async (t) => {
.getOnce(`https://api.github.local/repos/${owner}/${repo}`, {
permissions: {
push: true,
maintain: true,
},
clone_url: `https://api.github.local/${owner}/${repo}.git`,
})
Expand Down Expand Up @@ -303,6 +344,7 @@ test("Publish a release with release information in assets", async (t) => {
.getOnce(`https://api.github.local/repos/${owner}/${repo}`, {
permissions: {
push: true,
maintain: true,
},
clone_url: `https://api.github.local/${owner}/${repo}.git`,
})
Expand Down Expand Up @@ -376,6 +418,7 @@ test("Update a release", async (t) => {
.getOnce(`https://api.github.local/repos/${owner}/${repo}`, {
permissions: {
push: true,
maintain: true,
},
clone_url: `https://api.github.local/${owner}/${repo}.git`,
})
Expand Down Expand Up @@ -442,7 +485,10 @@ test("Comment and add labels on PR included in the releases", async (t) => {
.get(
`https://api.github.local/repos/${owner}/${repo}`,
{
permissions: { push: true },
permissions: {
push: true,
maintain: true,
},
full_name: `${owner}/${repo}`,
clone_url: `htttps://api.github.local/${owner}/${repo}.git`,
},
Expand Down Expand Up @@ -550,7 +596,10 @@ test("Open a new issue with the list of errors", async (t) => {
.get(
`https://api.github.local/repos/${owner}/${repo}`,
{
permissions: { push: true },
permissions: {
push: true,
maintain: true,
},
full_name: `${owner}/${repo}`,
clone_url: `htttps://api.github.local/${owner}/${repo}.git`,
},
Expand Down Expand Up @@ -645,7 +694,10 @@ test("Verify, release and notify success", async (t) => {
.get(
`https://api.github.local/repos/${owner}/${repo}`,
{
permissions: { push: true },
permissions: {
push: true,
maintain: true,
},
full_name: `${owner}/${repo}`,
clone_url: `htttps://api.github.local/${owner}/${repo}.git`,
},
Expand Down Expand Up @@ -811,7 +863,10 @@ test("Verify, update release and notify success", async (t) => {
.get(
`https://api.github.local/repos/${owner}/${repo}`,
{
permissions: { push: true },
permissions: {
push: true,
maintain: true,
},
full_name: `${owner}/${repo}`,
clone_url: `htttps://api.github.local/${owner}/${repo}.git`,
},
Expand Down Expand Up @@ -949,7 +1004,10 @@ test("Verify and notify failure", async (t) => {
.get(
`https://api.github.local/repos/${owner}/${repo}`,
{
permissions: { push: true },
permissions: {
push: true,
maintain: true,
},
full_name: `${owner}/${repo}`,
clone_url: `htttps://api.github.local/${owner}/${repo}.git`,
},
Expand Down
Loading

0 comments on commit be071a2

Please sign in to comment.