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
- 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
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