Skip to content

test: verify built-in token can resolve collaborator permission - #237

Closed
teeohhem wants to merge 2 commits into
mainfrom
test/collaborator-token-check
Closed

test: verify built-in token can resolve collaborator permission#237
teeohhem wants to merge 2 commits into
mainfrom
test/collaborator-token-check

Conversation

@teeohhem

Copy link
Copy Markdown
Contributor

Throwaway test workflow to verify that the built-in Actions GITHUB_TOKEN can resolve a user's repository permission — including access granted through a private team — under the exact permission scope that external-contributor-alerts.yml runs with.

This validates the planned fix for the false "external" classification of private org members (see the bug where a private MEMBER was flagged external because author_association only reflects public membership). The workflow runs on push and checks two logins:

  • teeohhem — a private ClickHouse member with write access. Expected verdict: INTERNAL.
  • octocat — no access. Expected verdict: EXTERNAL (control).

If the restricted token returns 403, the job fails loudly and we know a PAT/App token is required instead. This branch and file are temporary and will be deleted once the result is confirmed.


Compound Engineering
Claude Code

Throwaway workflow to confirm the Actions GITHUB_TOKEN can call the
collaborator-permission endpoint (and see private-team-derived access)
under the external-contributor-alerts permission scope. To be deleted.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@teeohhem
teeohhem requested a review from a team as a code owner June 29, 2026 20:44
@changeset-bot

changeset-bot Bot commented Jun 29, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 040654a

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions github-actions Bot added the external Opened by an external contributor label Jun 29, 2026
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@teeohhem

Copy link
Copy Markdown
Contributor Author

Test complete. Confirmed: built-in GITHUB_TOKEN resolves collaborator permission (no 403, no PAT needed); internal check must threshold on push/triage+ since public repos grant everyone implicit read. Closing throwaway.

@teeohhem teeohhem closed this Jun 29, 2026
@teeohhem
teeohhem deleted the test/collaborator-token-check branch June 29, 2026 20:48
@github-actions

Copy link
Copy Markdown
Contributor

Deep Review

✅ No critical issues found.

This PR adds one throwaway CI workflow (.github/workflows/test-collaborator-token-check.yml, 56 lines) that probes whether the built-in GITHUB_TOKEN can resolve collaborator permissions. It touches no Helm charts, values, or templates, so there is no rendered-manifest or deployment impact. Two recommended items below.

🟡 P2 -- recommended

  • .github/workflows/test-collaborator-token-check.yml:33 -- A specific login is hardcoded alongside a comment annotating it as a private org member with write access, and this is committed to a public repository's permanent git history.
    • Fix: Drive the check solely through the workflow_dispatch username input (not stored in history) or a non-identifying placeholder, and scrub the login via branch deletion/history rewrite rather than a later delete commit if it reaches the default branch.
    • ce-security-reviewer
  • .github/workflows/test-collaborator-token-check.yml:44 -- The verdict reads the nested data.user.permissions object, but the canonical resolved access for the team-granted case the experiment targets lives in the top-level data.permission / data.role_name; relying on the nested object risks a misleading verdict label.
    • Fix: Base the verdict on the documented top-level data.permission (e.g. include admin/write/triage/maintain) and keep the nested check only as a supplement.
    • ce-correctness-reviewer
🔵 P3 nitpicks (1)
  • .github/workflows/test-collaborator-token-check.yml:1 -- A self-described throwaway experiment workflow is committed to .github/workflows/ without a corresponding entry in the AGENTS.md CI Workflows table.
    • Fix: Run the experiment from a fork/local dispatch instead of committing it, or track its deletion explicitly so it does not become permanent dead workflow code.

Reviewers (4): ce-correctness-reviewer, ce-security-reviewer, ce-project-standards-reviewer, ce-maintainability-reviewer.

Testing gaps: Verdicts are only logged via core.info; the job does not assert the expected INTERNAL/EXTERNAL outcome per login, so a wrong-but-non-erroring classification passes silently. The permissions block grants issues: write + pull-requests: write though the script only reads collaborator permissions — intentional (it mirrors external-contributor-alerts.yml's token scope) and not reachable by untrusted actors, so left as a residual note rather than a finding.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

external Opened by an external contributor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant