github-community-projects / github-community-projects/safe-settings

Bug Report: environment and repo variables API calls don't paginate, failing with more than 10 items

Open Beginner friendly
#1,040 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
JavaScript
Stars
921
Forks
226
Avg merge
18h 3m
Merged PRs (30d)
14

Description

## Problem Description

### What is actually happening

`lib/plugins/environments.js` and `lib/plugins/variables.js` use `this.github.request()` to fetch variables, which returns only the first page of results. The GitHub API defaults to **10 items per page** for these endpoints, so any repository or environment with more than 10 variables is silently truncated.

During a sync, safe-settings is not able to validate the repo variables and fails with a "Variable already exists" error.

Reproduce by creating more than 10 vars in a repo

```
variables:
- name: var_name
value: value
- name: test_var_02
value: test_value_02
- name: test_var_03
value: test_value_03
- name: test_var_04
value: test_value_04
- name: test_var_05
value: test_value_05
- name: test_var_06
value: test_value_06
- name: test_var_07
value: test_value_07
- name: test_var_08
value: test_value_08
- name: test_var_09
value: test_value_09
- name: test_var_10
value: test_value_10
- name: test_var_11
value: test_value_11
- name: test_var_12
value: test_value_12
```

### What is the expected behavior

All existing variables should be fetched across all pages before diffing, so safe-settings correctly identifies which variables to add, update, or remove.

### Error output, if available

```
ERROR [17:29:23.216] (probot): Error HttpError: Already exists - Variable already exists - https://docs.github.com/rest/actions/variables#create-a-repository-variable in Variables for repo: {"owner":"demo","repo":"actions-test"} entries [{"name":"VAR_NAME","value":"value"},{"name":"TEST_VAR_02","value":"test_value_02"},{"name":"TEST_VAR_03","value":"test_value_03"},{"name":"TEST_VAR_04","value":"test_value_04"},{"name":"TEST_VAR_05","value":"test_value_05"},{"name":"TEST_VAR_06","value":"test_value_06"},{"name":"TEST_VAR_07","value":"test_value_07"},{"name":"TEST_VAR_08","value":"test_value_08"},{"name":"TEST_VAR_09","value":"test_value_09"},{"name":"TEST_VAR_10","value":"test_value_10"},{"name":"TEST_VAR_11","value":"test_value_11"},{"name":"TEST_VAR_12","value":"test_value_12"}]
```

## Context

### Are you using the hosted instance of probot/settings or running your own?

Running our own instance.

### If running your own instance, are you using it with github.com or GitHub Enterprise?

github.com

#### Version of probot/settings

2.1.21

#### Version of GitHub Enterprise

N/A

---

## Suggested Fix

Replace `this.github.request()` with `this.github.paginate()`, which is already used elsewhere in the codebase (e.g. `labels.js`, `milestones.js`, `rulesets.js`, `teams.js`).

**`lib/plugins/environments.js`** — in the `find()` method:
```js
// Before
variables: (await this.github.request('GET /repos/:org/:repo/environments/:environment_name/variables', {
org: this.repo.owner,
repo: this.repo.repo,
environment_name: environment.name
})).data.variables.map(variable => ({ name: variable.name.toLowerCase(), value: variable.value })),

// After
variables: (await this.github.paginate(
'GET /repos/{owner}/{repo}/environments/{environment_name}/variables',
{ owner: this.repo.owner, repo: this.repo.repo, environment_name: environment.name, per_page: 100 },
(response) => response.data.variables
)).map(variable => ({ name: variable.name.toLowerCase(), value: variable.value })),
```

**`lib/plugins/variables.js`** — in the `find()` method:
```js
// Before
return this.github.request('GET /repos/:org/:repo/actions/variables', {
org: this.repo.owner,
repo: this.repo.repo
}).then(({ data: { variables } }) => variables.map(({ name, value }) => ({ name, value })))

// After
return this.github.paginate(
'GET /repos/{owner}/{repo}/actions/variables',
{ owner: this.repo.owner, repo: this.repo.repo, per_page: 100 },
(response) => response.data.variables
).then(variables => variables.map(({ name, value }) => ({ name, value })))
```

Contributor guide

Open the contributing guide

Research direction

Start in the find() methods of lib/plugins/environments.js and lib/plugins/variables.js, then compare the pagination patterns already used in labels.js, milestones.js, rulesets.js, and teams.js. Done means both variable endpoints fetch all pages and a sync with more than 10 variables correctly diffs them without reporting an existing variable as new.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
api
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
85/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.