Skip to content

fix(security): authorize @gittensory Q&A commands by real repo permission (#788) - #838

Merged
JSONbored merged 1 commit into
mainfrom
feat/qa-command-real-permission
Jun 17, 2026
Merged

fix(security): authorize @gittensory Q&A commands by real repo permission (#788)#838
JSONbored merged 1 commit into
mainfrom
feat/qa-command-real-permission

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Closes #788. Agent-layer write-safety prerequisite.

The hole

The read-only @gittensory Q&A maintainer commands derived the actor's role from payload.comment.author_association (command-authorization.ts:139 maps org MEMBERmaintainer). Org MEMBER = mere membership in the owning org, not maintainer-of-this-repo. Low-stakes while read-only — but a privilege-escalation bug the moment @gittensory commands gain write power (#778: merge/close/review). The action-command path (#538) already resolves the real repo permission via getRepositoryCollaboratorPermission; the Q&A path didn't.

The fix

maybeProcessGittensoryMentionCommand now resolves the commenter's real repo permission (resolveRealRepoPermissionAssociation → admin/maintain→maintainer, write→collaborator, else none) inside the existing Promise.all, instead of trusting the spoofable author_association. An org member with only read access is no longer treated as a maintainer. Matches the action-command path and closes the hole before the agent-layer write actions land. No behavior change for genuine maintainers; PR-author / confirmed-miner paths unchanged.

Tests

  • New regression test: an org MEMBER with only read permission running @gittensory queue-summary is denied (not_maintainer_or_pr_author), and the real /collaborators/:login/permission lookup was consulted.
  • Updated the existing Q&A command tests to mock the real permission lookup (each command now makes one extra permission API call — token counts adjusted).

Verification

typecheck clean · full suite 1996 passed, 1 skipped (pre-existing pngjs visual-agent skip) · changed authorization path fully covered.

Relates #772 (Wave 2 — Agent Layer), #538, #778.

…sion, not author_association (#788)

The read-only @gittensory Q&A maintainer commands derived the actor's
role from payload.comment.author_association, which maps org `MEMBER`
(mere org membership) to the maintainer role — a privilege escalation
the moment those commands gain write power (#778: merge/close/review).
The action-command path (#538) already resolves the REAL repo permission
via getRepositoryCollaboratorPermission; the Q&A path did not.

Fix: maybeProcessGittensoryMentionCommand now resolves the commenter's
real repo permission (resolveRealRepoPermissionAssociation) instead of
trusting the spoofable author_association — an org member with only
read access is no longer treated as a maintainer. Matches the
action-command path and closes the hole before the agent-layer write
actions land.

Tests: added a regression test (org MEMBER + read-only permission →
denied with not_maintainer_or_pr_author); updated the existing Q&A
command tests to mock the real /collaborators/:login/permission lookup
(each command now makes one extra permission API call).
@dosubot dosubot Bot added the size:S label Jun 17, 2026
@ghost

ghost commented Jun 17, 2026

Copy link
Copy Markdown

Note

Gittensory Gate skipped

PR closed before full evaluation. No late first comment was created.

Signal Result Evidence Action
Gate result ⚠️ Skipped #838 is no longer open. No action.

💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

@ghost ghost added the gittensory:reviewed label Jun 17, 2026
@ghost

ghost commented Jun 17, 2026

Copy link
Copy Markdown

gittensory · advisory review

Reviewed 2 changed file(s) — two independent AI reviewers.

Changed files: src/queue/processors.ts, test/unit/queue.test.ts

Suggested action:Safe to merge — both reviewers found no blocking issues.

Reviewer A · gpt-oss-120b — recommends ✅ merge
The PR replaces the spoofable payload.author_association with a real GitHub permission check for @gittensory Q&A commands, tightening security, and adds comprehensive tests for the new flow and denial cases.

Suggestions

  • Verify that resolveRealRepoPermissionAssociation is imported in processors.ts to avoid compile errors.
  • Consider adding a test for handling a failed permission API call (e.g., network error or non‑200 response) to ensure graceful degradation.
  • Document the new permission check in the code comments or developer docs so future contributors understand the change.

Worth double-checking

  • The token count expectations in tests are tightly coupled to the number of external calls; future changes to permission logic could cause test failures.
  • If the permission endpoint returns unexpected values, ensure the authorization fallback remains safe.

Reviewer B · nemotron-3-120b-a12b — recommends ✅ merge
The PR fixes a privilege escalation vulnerability in @gittensory Q&A command authorization by replacing the spoofable payload.comment.author_association check with a real-time GitHub API call to verify the commenter's actual repository permission level. The change is narrow, well-tested, and aligns with the existing action-command path's security model. Tests are updated to reflect the new permission-check flow and include a dedicated test for the attack scenario.

Worth double-checking

  • The additional API call to resolveRealRepoPermissionAssociation may marginally increase latency and rate limit usage, but this is acceptable for security.
  • If the permission check fails due to network/GitHub API issues, commands will be safely skipped (existing error handling applies).

@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@codecov

codecov Bot commented Jun 17, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.56%. Comparing base (151435e) to head (f3979a7).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #838      +/-   ##
==========================================
- Coverage   96.56%   96.56%   -0.01%     
==========================================
  Files          98       98              
  Lines       14198    14197       -1     
  Branches     5175     5174       -1     
==========================================
- Hits        13711    13710       -1     
  Misses        105      105              
  Partials      382      382              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@JSONbored
JSONbored merged commit 207df8d into main Jun 17, 2026
20 checks passed
@JSONbored
JSONbored deleted the feat/qa-command-real-permission branch June 17, 2026 20:13
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.

[Bug/write-safety]: command authority conflates org MEMBER with repo maintainer (P1, agent-layer prereq)

1 participant