Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: Revert: feat: verify OAuth scopes of classic GitHub PATs #915

Merged
merged 1 commit into from
Sep 5, 2024
Merged

fix: Revert: feat: verify OAuth scopes of classic GitHub PATs #915

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 3 additions & 15 deletions lib/definitions/errors.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,24 +178,12 @@ 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 to and maintain the repository ${owner}/${repo}.`,
message: `The GitHub token doesn't allow to push on 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 have permission to push to and maintain the repository ${owner}/${repo}.
)}) configured in the \`GH_TOKEN\` or \`GITHUB_TOKEN\` environment variable must allows to push to 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 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.`,
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).`,
};
}

Expand Down
17 changes: 2 additions & 15 deletions lib/verify.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,21 +107,8 @@ export default async function verify(pluginConfig, context, { Octokit }) {
);
try {
const {
headers,
data: { private: _private, permissions, clone_url },
data: { 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 @@ -137,7 +124,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 && permissions?.maintain)) {
if (!env.GITHUB_ACTION && !permissions?.push) {
// 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: 5 additions & 63 deletions test/integration.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ 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 @@ -50,43 +49,6 @@ 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 @@ -100,7 +62,6 @@ 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 @@ -141,7 +102,6 @@ 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 @@ -248,7 +208,6 @@ 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 @@ -344,7 +303,6 @@ 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 @@ -418,7 +376,6 @@ 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 @@ -485,10 +442,7 @@ test("Comment and add labels on PR included in the releases", async (t) => {
.get(
`https://api.github.local/repos/${owner}/${repo}`,
{
permissions: {
push: true,
maintain: true,
},
permissions: { push: true },
full_name: `${owner}/${repo}`,
clone_url: `htttps://api.github.local/${owner}/${repo}.git`,
},
Expand Down Expand Up @@ -596,10 +550,7 @@ test("Open a new issue with the list of errors", async (t) => {
.get(
`https://api.github.local/repos/${owner}/${repo}`,
{
permissions: {
push: true,
maintain: true,
},
permissions: { push: true },
full_name: `${owner}/${repo}`,
clone_url: `htttps://api.github.local/${owner}/${repo}.git`,
},
Expand Down Expand Up @@ -694,10 +645,7 @@ test("Verify, release and notify success", async (t) => {
.get(
`https://api.github.local/repos/${owner}/${repo}`,
{
permissions: {
push: true,
maintain: true,
},
permissions: { push: true },
full_name: `${owner}/${repo}`,
clone_url: `htttps://api.github.local/${owner}/${repo}.git`,
},
Expand Down Expand Up @@ -863,10 +811,7 @@ test("Verify, update release and notify success", async (t) => {
.get(
`https://api.github.local/repos/${owner}/${repo}`,
{
permissions: {
push: true,
maintain: true,
},
permissions: { push: true },
full_name: `${owner}/${repo}`,
clone_url: `htttps://api.github.local/${owner}/${repo}.git`,
},
Expand Down Expand Up @@ -1004,10 +949,7 @@ test("Verify and notify failure", async (t) => {
.get(
`https://api.github.local/repos/${owner}/${repo}`,
{
permissions: {
push: true,
maintain: true,
},
permissions: { push: true },
full_name: `${owner}/${repo}`,
clone_url: `htttps://api.github.local/${owner}/${repo}.git`,
},
Expand Down
Loading