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.

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

visibility = "private" creates public repo first and then changes it to private #580

Closed
tprasadtp opened this issue Oct 31, 2020 · 9 comments
Labels
r/repository Type: Bug Something isn't working as documented

Comments

@tprasadtp
Copy link

tprasadtp commented Oct 31, 2020

I have noticed that when creating repositories, when visibility is set to private, repositories are created public by default and then changed to private at a later point. Is this expected? If so, Shouldn't it be documented? It can potentially make private code public if used in conjunction with a private template repository. In case of network failure, where the resourceGithubRepositoryUpdate never gets completed, I know, I might have hit a super edge case with this one..

Terraform Version

Terraform v0.13.5

  • provider registry.terraform.io/hashicorp/github v3.1.0

Affected Resource(s)

  • github_repository

Terraform Configuration Files

resource "github_repository" "repo" {
  name        = "repo"
  description = "Must be Private Repo"
  visibility = "private"
  homepage_url = "https://homepage.service.consul"

  template {
    owner = "owner-org"
    repository = "private-template"
  }

  #Enable issues
  has_issues = true

  # disable others
  has_wiki = false
  has_projects = false

  # Not a template
  is_template = false

  # PR Settings
  allow_merge_commit = false
  allow_squash_merge = true
  allow_rebase_merge = false
  delete_branch_on_merge = true
}

Expected Behavior

Repositories should be created as private

Actual Behavior

Repository is created as public and then changed to private, If its desired behavior, it should be documented.

@jcudit
Copy link
Contributor

jcudit commented Nov 4, 2020

Thanks for raising this and getting this behaviour more visible.

I think a documentation PR makes sense in the short term and also welcome a fix that doesn't rely on the update sequence to make a repo private.

Not sure when we can correct this behaviour though, but will leave this issue open for others to chime in on priority.

@genesor
Copy link

genesor commented Nov 12, 2020

I encountered the same problem while trying to terraform repo creation.

I noticed that there is a mistake in the comment on the repository creation logic here

You do not need to be an admin in an organization to create a repository, members can do it referring to GH doc, but you need to be an admin to change the visibility settings.

This makes private repository creation impossible if you do not run the terraform via an organization owner, which is what we tried to do.

PATCH https://api.github.com/repos/XXXX/XXXX: 422 Visibility can't be changed by this user. and Visibility can't be changed by this user. []

@majormoses
Copy link
Contributor

I will try to find some time to take a look at this, we do run our bot with org admin for other reasons even if we scoped the API tokens down in certain areas.

@majormoses
Copy link
Contributor

To further clarify we prevent users from creating repositories without being admin (be they public or not). We do allow this specific bot admin access but we also scope certain actions to not be available with the access token associated with the user.

I agree the the behavior described is clearly undesirable but I am not sure what attack vectors may exist that I have not thought through. In most cases this would not cause any concern as they would be an empty repository. The one concern that I do have is initializing a repository from a private template.

While it is generally a good idea to give users as few permissions as possible I decided at my organization that we needed to focus on protecting the user, service, and scope out things that we can't live with a bug doing. For example we do not allow terraform to delete a repository and require an admin to remove it and create a pull request to prevent it from being recreated. Taking this approach I was able to reduce the number of admins by probably close to 50 people and that is a real security win. Systems such as terraform, chef, puppet, ansible, etc running as highly privileged access is not inherently bad as it means you can turn up your sensitivity to potentially dangerous behavior that falls outside of that and you should be guarding those pipelines carefully.

I think we want to start with a documentation PR so that we can raise awareness and then we can decide how to proceed with a change.

@jcudit
Copy link
Contributor

jcudit commented Dec 10, 2020

Still needs more experimentation, but it looks like the GraphQL API has the ability to create a repository with any visibility without using a subsequent update. Not sure if non-admin members would be able to leverage this approach.

/cc https://docs.github.com/en/free-pro-team@latest/graphql/reference/enums#repositoryvisibility
/cc https://docs.github.com/en/free-pro-team@latest/graphql/reference/input-objects#createrepositoryinput

@nantiferov
Copy link

I got the same error and had to change private = true to visibility = "private".
But this is deprecated, so I'm not sure how long it will work.

I'm not able to create public repos with my token, so it would be really great to be able to create private repos without this intermediate step with public repo creation.

@ap0phi5
Copy link

ap0phi5 commented Jan 19, 2021

Not got round to testing this yet, but I'm pretty certain this will probably also hit our GitHub Enterprise Cloud policy where public repos are disabled. This definitely affects us, so if I get time, I'll take a look.

@jurgen-weber-deltatre
Copy link

#680

fix here, I didn't think a repo's visibility could be changed. Do we know what scenario's it can be changed?

@nantiferov
Copy link

Shouldn't this issue be fixed by #746 ?

@jcudit jcudit closed this as completed Jun 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
r/repository Type: Bug Something isn't working as documented
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants