README.md recommends an insecure default configuration

[ramblingenzyme]

Now that OAuth clients are supported, I think the recommended configuration in the README.md should show an OAuth configuration rather than an API key based one.

I also think it should suggest that you should configure an OAuth client with acl:read scope when running tests, and a second one with acl for applying updates. This prevents an escalation path where the ACLs can be updated from an arbitrary branch by updating the workflow, i.e.

name: Sync Tailscale ACLs

on:
  push:
    branches: [ "main" ]
  pull_request:
    branches: [ "main" ]

jobs:
  acls:
    runs-on: ubuntu-latest

    steps:
      ...
      - name: Deploy ACL
        # By commenting out this `if`, the ACLs get applied when the workflow runs on the PR trigger.
        # if: github.event_name == 'push'
        id: deploy-acl
        uses: tailscale/gitops-acl-action@v1
        with:
          api-key: ${{ secrets.TS_API_KEY }}
          tailnet: ${{ secrets.TS_TAILNET }}
          action: apply
      ...

For reference, this is the workflow we're using, and how we have the secrets configured.

name: Sync Tailscale ACLs

on:
  push:
    branches: [ "main" ]
  pull_request:
    branches: [ "main" ]

jobs:
  apply-acls:
    if: github.event_name == 'push'
    environment: production
    runs-on: ubuntu-latest

    steps:
      - uses: actions/checkout@v3

      - name: Fetch version-cache.json
        uses: actions/cache@v3
        with:
          path: ./version-cache.json
          key: version-cache.json-${{ github.run_id }}
          restore-keys: |
            version-cache.json-
      - name: Deploy ACLs
        id: deploy-acls
        # Tailscale has released OAuth support for their action, but haven't cut a new release yet
        uses: tailscale/gitops-acl-action@287fb935799def5f8a2aef4df9b1286f78fc384b
        with:
          oauth-client-id: ${{ secrets.TS_OAUTH_CLIENT_ID }}
          oauth-secret: ${{ secrets.TS_OAUTH_SECRET }}
          tailnet: ${{ secrets.TS_TAILNET }}
          action: apply

  test-acls:
    if: github.event_name == 'pull_request'
    runs-on: ubuntu-latest

    steps:
      - uses: actions/checkout@v3

      - name: Fetch version-cache.json
        uses: actions/cache@v3
        with:
          path: ./version-cache.json
          key: version-cache.json-${{ github.run_id }}
          restore-keys: |
            version-cache.json-
      - name: Test ACLs
        id: test-acls
        # Tailscale has released OAuth support for their action, but haven't cut a new release yet
        uses: tailscale/gitops-acl-action@287fb935799def5f8a2aef4df9b1286f78fc384b
        with:
          oauth-client-id: ${{ secrets.TS_OAUTH_CLIENT_ID }}
          oauth-secret: ${{ secrets.TS_OAUTH_SECRET }}
          tailnet: ${{ secrets.TS_TAILNET }}
          action: test

[rowanmoul]

I'm not sure I follow your solution to privilege escalation in PRs. What is to stop me from making any number of other changes to the yaml in a PR that results in acl changes being applied before merging to main? I think the solution is to just not allow anyone untrusted to make PRs in the repository in the first place.

Edit: I was not framiliar with "environments". I see there is a way to limit their use to specific branches, which would indeed prevent malicious changes to the yaml from being applied.

[gabeio]

@rowanmoul yes you can limit access to an environment based on a branch:

screenshot

Typically over in the repo settings under Environments > select your environment and you will be able to limit the environment to a branch or branches/tags/etc.

https://github.com/{user_or_org}/{repo}/settings/environments/{environment_id}/edit