Skip to content

fix: use repo-scoped token for pulls.get in Check if org member step#195

Merged
derekmisler merged 7 commits into
docker:mainfrom
docker-agent:fix/org-member-repo-token
May 8, 2026
Merged

fix: use repo-scoped token for pulls.get in Check if org member step#195
derekmisler merged 7 commits into
docker:mainfrom
docker-agent:fix/org-member-repo-token

Conversation

@docker-agent
Copy link
Copy Markdown
Contributor

Problem

The Check if org member step passes ORG_MEMBERSHIP_TOKEN as the github-token to actions/github-script. This token only has read:org scope — it does not have repo scope.

When PR_SOURCE is trigger (the workflow_run path), the script falls into the else branch and calls github.rest.pulls.get() using that same token. Because the token lacks repo scope, GitHub returns 404, crashing the step with HttpError: Not Found.

Fix

Two minimal, targeted changes in the Check if org member step of .github/workflows/review-pr.yml:

  1. Added REPO_TOKEN to the step's env block — resolves to GITHUB_APP_TOKEN (which has full repo access) when available, falling back to github.token:

    REPO_TOKEN: ${{ env.GITHUB_APP_TOKEN || github.token }}
  2. Created a secondary octokit client for the pulls.get call, using REPO_TOKEN instead of the org-membership-scoped github client:

    const repoOctokit = github.getOctokit(process.env.REPO_TOKEN);
    const { data: pr } = await repoOctokit.rest.pulls.get({ ... });

The primary github client (backed by ORG_MEMBERSHIP_TOKEN) is kept exclusively for checkMembershipForUser — no other logic was changed.

Validation

pnpm lint passes cleanly: Biome CI (69 files, no issues), tsc --noEmit (no errors), actionlint (no errors).

The ORG_MEMBERSHIP_TOKEN only has read:org scope — it lacks repo scope.
When PR_SOURCE is 'trigger' (workflow_run path), the else branch calls
github.rest.pulls.get() using that token, which returns 404 and crashes
the step with HttpError: Not Found.

Fix: add REPO_TOKEN env var (GITHUB_APP_TOKEN or github.token, both of
which carry repo scope) and create a secondary octokit client with it
exclusively for the pulls.get call. The primary github client (backed by
ORG_MEMBERSHIP_TOKEN) is kept for checkMembershipForUser only.
@docker-agent docker-agent requested a review from a team as a code owner May 8, 2026 12:57
@docker-agent docker-agent requested a review from derekmisler May 8, 2026 12:57
Copy link
Copy Markdown
Contributor Author

@docker-agent docker-agent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assessment: 🟢 APPROVE

Fix is correct and well-scoped.

The PR correctly addresses the root cause: ORG_MEMBERSHIP_TOKEN only has read:org scope and cannot call pulls.get (which requires repo scope). The fix introduces a separate repoOctokit via github.getOctokit(process.env.REPO_TOKEN) using a token with sufficient scope, while preserving ORG_MEMBERSHIP_TOKEN for the checkMembershipForUser call.

Verified:

  • actions/github-script is pinned to v9.0.0github.getOctokit() is available ✅
  • REPO_TOKEN: ${{ env.GITHUB_APP_TOKEN || github.token }}github.token is always non-empty, so REPO_TOKEN is always populated ✅
  • setup-credentials runs immediately before this step and exports GITHUB_APP_TOKEN, so the preferred high-privilege token is used when available ✅
  • The ${{ env.GITHUB_APP_TOKEN || github.token }} pattern is already established in 5+ other steps in the same workflow ✅

…KEN only

Per security design, GITHUB_APP_TOKEN is the PAT fetched from AWS Secrets
Manager and must be the sole token for non-membership operations. A silent
fallback to github.token is not acceptable — the setup-credentials step
already guarantees GITHUB_APP_TOKEN is present before this step runs.
- Add resolvePrAuthor(repoToken, owner, repo, prNumber) to check-org-membership
  module: fetches PR author via pulls.get using a repo-scoped token, keeping
  the token split (ORG_MEMBERSHIP_TOKEN only for checkMembershipForUser,
  GITHUB_APP_TOKEN for pulls.get) as a first-class concern in the type-safe layer.

- Add CLI entry point (main()) guarded by process.argv[1] filename check so
  the module continues to work as a library imported by mention-reply and main/auth
  without triggering the CLI code when bundled into those distributions.

- Add check-org-membership to tsup entry map so the module gets its own
  dist/check-org-membership.js standalone bundle.

- Add .github/actions/check-org-membership/action.yml (node24 action) that
  wraps the bundle with well-typed inputs (org-membership-token, github-app-token,
  org, pr-source, pr-number, comment-author) and output (is_member).

- Extend unit tests: resolvePrAuthor happy/sad paths and a token-isolation test
  that asserts the two Octokit instances receive their respective tokens in order.

The follow-up commit will update review-pr.yml to use this action via
docker/cagent-action/.github/actions/check-org-membership@<this-sha>.
…ction

Wire up the new .github/actions/check-org-membership (node24) in review-pr.yml,
replacing the inline actions/github-script block. The step now delegates to
dist/check-org-membership.js via explicit inputs, matching the project convention
of keeping business logic in tested TypeScript modules rather than inline YAML JS.

Token split is enforced at the action boundary:
  org-membership-token -> checkMembershipForUser (read:org scope)
  github-app-token     -> resolvePrAuthor / pulls.get (repo scope)

SHA points to the commit that introduced the action file and bundle entry
(e9a0c6a); the release workflow will
update all internal SHA pins together when the next version is cut.
Remove the .github/actions/check-org-membership/ wrapper — no separate action
file needed. The bundle is now invoked directly as a shell run step, consistent
with how review-pr/action.yml calls other dist/ tools.

Changes:
- setup-credentials/action.yml: export CAGENT_ACTION_ROOT (the repo root of the
  downloaded cagent-action copy) to the job environment in the same 'Verify
  credentials' step. Subsequent steps in the calling workflow can then reach
  dist/ bundles via $CAGENT_ACTION_ROOT/dist/<name>.js.
- review-pr.yml 'Check if org member': replace the uses: action invocation with
  a plain 'run: node "$CAGENT_ACTION_ROOT/dist/check-org-membership.js"' step;
  inputs passed via env vars (ORG, PR_SOURCE, PR_NUMBER, COMMENT_AUTHOR);
  ORG_MEMBERSHIP_TOKEN and GITHUB_APP_TOKEN are already in the job env from
  setup-credentials.
- src/check-org-membership/index.ts: CLI reads from process.env (not
  core.getInput) since it is invoked via a shell run step, not a node action.
Promotes setup-credentials from .github/actions/setup-credentials/ to
setup-credentials/ so external consumers can reference it as:

  uses: docker/cagent-action/setup-credentials@VERSION

matching the pattern already established by review-pr/ and consistent with
how other repos already use it. The .github/actions/ location prevented
clean external references.

mention-reply stays under .github/actions/ — it is only called from within
review-pr.yml and is not intended for external use.

With setup-credentials at the root, $GITHUB_ACTION_PATH inside the action
is one level deep, so the credentials bundle path simplifies from
$GITHUB_ACTION_PATH/../../../dist/credentials.js
to
$GITHUB_ACTION_PATH/../dist/credentials.js

and the CAGENT_ACTION_ROOT export from:
  cd "$GITHUB_ACTION_PATH/../../.." && pwd
to:
  cd "$GITHUB_ACTION_PATH/.." && pwd

All uses: references updated across all workflow files.
$ACTION_PATH is set from ${{ github.action_path }} which points to the
review-pr/ subdirectory, not the repo root. So $ACTION_PATH/dist/ resolved
to review-pr/dist/ (non-existent) rather than the root dist/.

Fix both bundle calls to use $ACTION_PATH/../dist/, matching the same
one-level-up pattern that setup-credentials/action.yml already uses
($GITHUB_ACTION_PATH/../dist/credentials.js).
@derekmisler derekmisler marked this pull request as draft May 8, 2026 13:58
@derekmisler derekmisler marked this pull request as ready for review May 8, 2026 13:58
@derekmisler
Copy link
Copy Markdown
Contributor

/review

@derekmisler derekmisler merged commit 4f5e1f3 into docker:main May 8, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants