Skip to content

feat: require explicit admin override for self-approval; fix :run PID file#216

Merged
coopernetes merged 1 commit into
mainfrom
feat/admin-toggle-self-approve
May 4, 2026
Merged

feat: require explicit admin override for self-approval; fix :run PID file#216
coopernetes merged 1 commit into
mainfrom
feat/admin-toggle-self-approve

Conversation

@coopernetes
Copy link
Copy Markdown
Member

Summary

  • Admins reviewing their own pushes now follow the same path as regular users — ROLE_ADMIN alone no longer bypasses the self-approval identity check. Admins either self-certify (via ROLE_SELF_CERTIFY + repo permission, same as any user) or must explicitly activate an Enable admin override toggle in the dashboard UI for break-glass situations.
  • The admin override toggle is hidden when self-certify is already available, so the aggressive red warning only surfaces when it is actually needed.
  • selfApproval=true is only recorded in the audit log when the override flag is explicitly used — admin self-certify approvals are no longer incorrectly flagged.
  • Fix :run PID file: applicationDefaultJvmArgs only applies to generated distribution scripts, not the Gradle JavaExec run task. Added -Dgitproxyjava.pidfile explicitly to tasks.named('run') in both build.gradle files so ./gradlew :stop works correctly after ctrl+c.
  • Local dev config: grant thomas-cooper ROLE_SELF_CERTIFY so the self-certify path is exercisable without needing the admin override.

Test plan

  • Log in as an admin user who pushed a commit — confirm blue self-certify banner appears (not red override warning)
  • Log in as an admin without self-certify permission — confirm amber "not permitted" banner and "Enable admin override" link
  • Click "Enable admin override" — confirm red warning appears and approve button becomes active
  • Approve via override — confirm audit log entry has selfApproval: true
  • Approve via self-certify — confirm audit log entry has selfApproval: false
  • Admin approving someone else's push — confirm no change in behaviour (bypass still applies)
  • ./gradlew :git-proxy-java-dashboard:run then ctrl+c then ./gradlew :git-proxy-java-dashboard:stop — confirm "Stopping … (PID: …)" rather than "No PID file found"
  • ./gradlew :git-proxy-java-dashboard:test passes

… file

Admin users now follow the same self-approval path as regular users when
reviewing their own pushes. ROLE_ADMIN alone no longer bypasses the
identity check for self-review — admins either use the self-certify path
(ROLE_SELF_CERTIFY + repo permission) or must explicitly activate an
admin override toggle in the dashboard UI.

Changes:
- Backend: checkReviewerIdentity now treats admin self-review the same
  as regular user self-review unless adminOverride=true is sent in the
  approve request body. Admins reviewing someone else's push still bypass
  unconditionally. isSelfApproval only flags the attestation when the
  override is explicitly used.
- Frontend: isSelfReview applies to admins too; self-certify blue banner
  shown for admins with the permission; admin override toggle is hidden
  when self-certify is active and only surfaces as a low-prominence link
  for the break-glass case.
- Fix :run PID file: applicationDefaultJvmArgs only applies to generated
  distribution scripts, not the Gradle JavaExec run task. Added the
  -Dgitproxyjava.pidfile JVM arg explicitly to tasks.named('run') in
  both server and dashboard build.gradle so :stop works after ctrl+c.
- Local config: grant thomas-cooper ROLE_SELF_CERTIFY so the self-certify
  path is exercisable in local dev without needing the admin override.

closes #184

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@coopernetes coopernetes enabled auto-merge May 4, 2026 14:37
@coopernetes coopernetes merged commit 4e9cd82 into main May 4, 2026
13 of 16 checks passed
@coopernetes coopernetes deleted the feat/admin-toggle-self-approve branch May 4, 2026 14:40
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.

1 participant