checkAllowedRepo() ignores handler's target-repo, causing spurious post-success failure after create_pull_request
- Dominant language
- JavaScript
- Stars
- 47
- Forks
- 19
- Avg merge
- 6m
- Merged PRs (30d)
- 24
Description
## Summary
`setup/js/invocation_context_helpers.cjs`'s `checkAllowedRepo()` — invoked via `resolveInvocationContext()` from `updateActivationComment()` right after a successful `create_pull_request` — can spuriously fail on a repo that the `create_pull_request` safe-output handler is explicitly configured to target, causing a confusing post-success error even though the PR was created correctly.
## Impact
Any compiled workflow whose `create_pull_request` safe-output handler is configured with only a `target-repo` field (no separate `allowed_repos` list) will hit this: the PR is created successfully, but the subsequent activation-comment step throws:
```
Error: ERR_VALIDATION: Repository 'github/' is not in the allowed-repos list. Allowed:
at checkAllowedRepo (invocation_context_helpers.cjs:193:25)
at resolveInvocationContext (invocation_context_helpers.cjs:239:9)
at updateActivationComment (update_activation_comment.cjs:28:29)
at handleCreatePullRequest (create_pull_request.cjs:2734:15)
```
This makes the job/run report a failure even though the actual safe output (the PR) succeeded, which is misleading and breaks automation that checks job status.
## Root cause
`checkAllowedRepo()`'s fallback path (used when no global `GH_AW_ALLOWED_REPOS` env var is set) builds its allowlist by scanning each safe-output handler's config for an `allowed_repos` array field only. It never considers a handler's `target-repo` field as an implicitly allowed repo, even though a completely separate and correct validation path — `resolveAndValidateRepo()` in `repo_helpers.cjs` — *does* treat `target-repo` as the default allowed repo. This inconsistency between the two validation paths is what causes the spurious failure: the PR creation itself passes validation (via `repo_helpers.cjs`), but the later activation-comment step re-validates using the other, stricter path (`invocation_context_helpers.cjs`) which doesn't know about `target-repo`.
## Fix
We patched this internally by adding the handler's `target-repo` value to the allowlist `Set` in the same per-handler fallback loop that already handles `allowed_repos`:
```js
if (typeof value["target-repo"] === "string" && value["target-repo"].trim() !== "") {
allowedRepos.add(value["target-repo"].trim());
}
```
Fixed in our internal fork at commit `2491aee56d1c7aed4ec733f415a623d2c2e14e7a` (branch `fix/gh-host-env-clobber`, stacked on the `runGH()`/`GH_HOST` fix). Happy to open a PR with this fix upstream if useful.
## How we found it
Static analysis of the vendored/bundled JS across `create_pull_request.cjs`, `repo_helpers.cjs`, `update_activation_comment.cjs`, and related files didn't reveal an obvious throw site matching the error text. We confirmed the exact call stack by temporarily adding `error.stack` logging to the relevant catch blocks and re-running the workflow live.
## Environment
Discovered running a `create_pull_request` safe-output handler configured with only `target-repo` (no `allowed_repos`) against a GitHub Enterprise Cloud tenant with data residency (a `.ghe.com` host).
Contributor guide
Research direction
Start in setup/js/invocation_context_helpers.cjs at checkAllowedRepo(), then trace its use through resolveInvocationContext() and update_activation_comment.cjs. Compare the fallback allowlist handling with resolveAndValidateRepo() in repo_helpers.cjs. Done means a handler configured only with target-repo passes the post-success validation without weakening existing allowed_repos checks.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript
- Domain
- tooling
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 86/100