github-community-projects / github-community-projects/safe-settings
Bypass actors with `actor_id: null` are invisible to ruleset diffs; `actor_id: 1` re-applies forever
- Dominant language
- JavaScript
- Stars
- 921
- Forks
- 226
- Avg merge
- 18h 3m
- Merged PRs (30d)
- 14
Description
For ruleset `bypass_actors`, GitHub's API ignores `actor_id` when `actor_type` is `OrganizationAdmin` (or `DeployKey`) and always returns `actor_id: null` for those entries. `MergeDeep` cannot handle that shape, in either direction:
**Configured as `actor_id: null`** (matching what the API returns): the entry never deploys. `NAME_USERNAME_PROPERTY` picks `actor_id` as the object's identity field because the key exists, `GET_NAME_USERNAME_PROPERTY` returns the null, and `processArrays` skips entries with a falsy id (`if (id) visited[id] = a`). An `OrganizationAdmin` actor present in config but missing from the live ruleset produces no addition, no modification, and a "no changes" plan. The sync silently does nothing.
**Configured as `actor_id: 1`** (what `docs/sample-settings/settings.yml` shows): the entry deploys, but on the next sync the config entry (id `1`) cannot match the live entry (id `null`), so the same change is detected and re-applied on every run, forever. In our org this flapped 15 repos on an hourly schedule until we noticed.
So there is no `actor_id` value for an `OrganizationAdmin` bypass actor that both deploys and converges.
Reproduction with `compareDeep` directly:
```js
const md = new MergeDeep(log, github, [])
const orgAdmin = { actor_id: null, actor_type: 'OrganizationAdmin', bypass_mode: 'pull_request' }
md.compareDeep({ bypass_actors: [] }, { bypass_actors: [orgAdmin] }, {}, {}, {})
// => hasChanges: false (expected: an addition)
```
Contributor guide
Research direction
Start by reading MergeDeep's NAME_USERNAME_PROPERTY, GET_NAME_USERNAME_PROPERTY, and processArrays paths, then run the compareDeep reproduction from the issue. Check the behavior against the OrganizationAdmin entry shown in docs/sample-settings/settings.yml. Done means null-identified bypass actors produce the expected change and repeated syncs converge.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- github, javascript
- Domain
- api, backend
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 58/100