Skip to content

fix(github): classify runtime authorization denial reasons - #6182

Closed
pandemicsyn wants to merge 2 commits into
mainfrom
investigate/github-runtime-auth-20260915
Closed

pandemicsyn wants to merge 2 commits into
mainfrom
investigate/github-runtime-auth-20260915

Conversation

@pandemicsyn

@pandemicsyn pandemicsyn commented Sep 15, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Adds bounded reporting for actionable GitHub runtime authorization denials at the standard webhook route, the deferred webhook dispatch check, and the bot-session callback: a fixed warning-level message carrying a single bounded reason tag.
  • missing_association reporting behavior at those callers is retained unchanged; the exact production cause of those denials remains unproven. Every other rejection reason reports.
  • Current main already contains the detailed rejection taxonomy and the prepareReviewPayload diagnostics, so this PR does not change or duplicate them. The only new code is a shared predicate plus the three caller report sites and their tests.
  • Diagnostics only: the authorization and ownership decision, the fail-closed behavior, the webhook acknowledgment responses, the deferred-dispatch skip, and the callback terminal outcome and message are unchanged. No installation lifecycle, persistence, schema, or migration change.
  • Merged current main into this branch to resolve conflicts before review. Related to KILOCODE-WEB-27W0 only.

Verification

No manual verification was performed. The change is confined to server-side error classification and reporting plus tests; no user-facing or interactive path was exercised in this session.

Visual Changes

N/A

Reviewer Notes

  • Strong overlap with open feat(github): share installations with web Cloud Agents  #5978, feat(slack): support shared GitHub Cloud Agent workflows #6089, and fix(github): fix connect/disconnect lifecycle bugs found in live org testing #6100. When integrating, preserve their exact association-ID authorization, canonical lifecycle checks, and shared-tenant routing; port only this PR's boundary reporting and never replace a stack runtime guard with an older authorization model.
  • The shared predicate selects the reported reasons and is intentionally bounded. Confirm missing_association retains its existing behavior at these callers while the actionable reasons report; the exact production cause of missing-association denials is not established by this change.
  • Confirm the authorization decision, the webhook acknowledgment (200 for authorization denials, 503 only for non-authorization failures), the deferred-dispatch skip, and the callback terminal outcome are unchanged.
  • The added tag carries only the bounded reason enum; no identifiers, credentials, raw errors, or request payloads are added.
  • Ready against current main after merge resolution. Not verified as composable with the unmerged stack PRs; that integration is a follow-up decision.

Attach a bounded denial reason to GitHub runtime authorization failures and report actionable denial categories at the webhook and bot-callback boundaries. Diagnostics only: authorization decisions, fail-closed behavior, webhook acknowledgements, and callback terminal outcomes are unchanged.

Related: KILOCODE-WEB-27W0
Resolve conflicts in runtime authorization and review payload diagnostics by adopting the upstream rejection taxonomy and payload diagnostics, keeping only the caller-boundary denial reporting added by this branch.

No behavior change to authorization, webhook acknowledgements, deferred dispatch, or callback terminal outcomes.
@pandemicsyn
pandemicsyn marked this pull request as ready for review September 15, 2026 19:59
@kilo-code-bot

kilo-code-bot Bot commented Sep 15, 2026 •

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Files Reviewed (10 files)
  • apps/web/src/lib/integrations/github/runtime-authorization.ts
  • apps/web/src/lib/integrations/github/runtime-authorization.test.ts
  • apps/web/src/app/api/webhooks/github/route.ts
  • apps/web/src/app/api/webhooks/github/route.test.ts
  • apps/web/src/lib/integrations/platforms/github/webhook-handler.ts
  • apps/web/src/lib/integrations/platforms/github/webhook-handler.test.ts
  • apps/web/src/app/api/internal/bot-session-callback/[botRequestId]/route.ts
  • apps/web/src/app/api/internal/bot-session-callback/[botRequestId]/route.test.ts
  • apps/web/src/lib/integrations/db/github-installations.test.ts
  • apps/web/src/lib/code-reviews/triggers/prepare-review-payload.test.ts

Statically reviewed isUnexpectedGitHubRuntimeAuthorizationDenial plus its three call-site report blocks and all touched tests. The predicate only adds bounded captureMessage tags (source/endpoint + github_runtime_authorization_reason) and leaves authorization decisions, fail-closed behavior, acknowledgement responses, deferred-dispatch skips, and callback terminal outcomes unchanged; no leaked identifiers or credentials and no listener/timer additions. The ReadonlySet allowlist mirrors existing codebase patterns, so it was not raised. The custom kilocode-landing page rules are N/A to this repository. Test assertions (reason ordering such as integration_status before disconnected, and the ambiguity case) match the current implementation.


Reviewed by deepseek-v4.1-flash · Input: 0 · Output: 0 · Cached: 0

Review guidance: REVIEW.md from base branch main

@pandemicsyn pandemicsyn closed this Oct 1, 2026
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