Skip to content

fix(auth): require repo write access for activation - #711

Merged
JSONbored merged 2 commits into
mainfrom
codex/propose-fix-for-advisory-checks-vulnerability
Jun 14, 2026
Merged

fix(auth): require repo write access for activation#711
JSONbored merged 2 commits into
mainfrom
codex/propose-fix-for-advisory-checks-vulnerability

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Motivation

  • Prevent read-only org members/collaborators who are only inferred as maintainers via PR author_association from enabling repo-visible advisory gate behavior.
  • Activation currently persisted settings that drive GitHub check-run behavior while only using the weaker maintainer DATA guard, allowing unauthorized state changes.

Description

  • Require live repository write permission before persisting activation by replacing requireRepoMaintainer with requireRepoWriteAccess on POST /v1/repos/:owner/:repo/activation and by generalizing the stricter guard for other write endpoints.
  • Generalized the BYOK write guard into requireRepoWriteAccess (renamed REPO_KEY_WRITE_PERMISSIONSREPO_WRITE_PERMISSIONS) and updated ai-key endpoints to use it so both repo-visible settings writes and secret writes require live GitHub admin/maintain/write permission.
  • Added integration tests that mock getRepositoryCollaboratorPermission and seed an installation to assert a read-only collaborator (PR author association COLLABORATOR with read permission) can preview activation but receives 403 on activation, and that a session with GitHub write permission can activate successfully.

Testing

  • Ran the focused integration and unit tests: npm test -- --run test/integration/maintainer-activation.test.ts test/unit/routes-ai-byok.test.ts, and all tests passed (21 tests, 21 passed).
  • Ran type checking with npm run typecheck, which succeeded with no TypeScript errors.
  • Verified diffs with git diff --check for the modified files and committed the change.

Codex Task

@ghost

ghost commented Jun 14, 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 #711 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 14, 2026
@ghost

ghost commented Jun 14, 2026

Copy link
Copy Markdown

gittensory · advisory review

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

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

Reviewer A · gpt-oss-120b — recommends ✅ merge
The PR tightens activation and AI‑key routes by requiring real GitHub write permissions instead of just maintainer status, renames the related constant and guard function, and adds comprehensive integration tests for permission checks.

Suggestions

  • Run the full TypeScript build to verify no remaining references to the old requireRepoKeyWriteAccess or REPO_KEY_WRITE_PERMISSIONS exist elsewhere in the codebase.
  • Consider adding a test case for operator (ADMIN_GITHUB_LOGINS) bypass to ensure the new guard still allows privileged accounts.

Worth double-checking

  • Potential compile errors if other modules still import the renamed guard or constant.
  • The new permission check relies on getRepositoryCollaboratorPermission; ensure its mock behavior matches production for edge cases.

Reviewer B · nemotron-3-120b-a12b — recommends ✅ merge
The PR tightens permission checks for repo activation and AI-key routes by replacing requireRepoMaintainer with a new requireRepoWriteAccess helper that enforces real GitHub write/admin/maintain permissions. It also renames the permission constant and updates accompanying comments. The integration test file is expanded with mocks and new test cases covering read-only collaborator denial and write permission allowance. Overall the change is a clean refactor that aligns the gates with the intended public/private boundary for repo-visible settings and secret BYOK writes.

Suggestions

  • Consider adding a test case to verify that operator/admin sessions (ADMIN_GITHUB_LOGINS) bypass the per-repo write check, to future-proof against regressions.
  • If any other internal callers of the old requireRepoKeyWriteAccess exist outside the diff, ensure they are updated; otherwise the rename is safe.
  • Optionally add a brief comment explaining why the constant was renamed (to reflect broader repo-visible settings scope).

Worth double-checking

  • Verify that no other routes still rely on the old requireRepoKeyWriteAccess name (the diff shows all three call sites updated).
  • Ensure the requireRepoWriteAccess function correctly handles server-to-server tokens and operator exemptions as the previous version did.
  • Confirm that the updated comment accurately reflects the new scope (repo-visible settings + BYOK writes) and does not misrepresent the gate.

@superagent-security

Copy link
Copy Markdown
Contributor

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

@ghost ghost added the gittensory-review label Jun 14, 2026
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jun 14, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
gittensory-ui 09ecfc7 Commit Preview URL

Branch Preview URL
Jun 14 2026, 02:16 PM

@JSONbored
JSONbored merged commit 26aee37 into main Jun 14, 2026
11 checks passed
@JSONbored
JSONbored deleted the codex/propose-fix-for-advisory-checks-vulnerability branch June 14, 2026 14:26
@github-project-automation github-project-automation Bot moved this from Todo to Done in gittensory - v1 roadmap Jun 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant