fix(ai-review): harden BYOK — provider timeout, rate-limit, maintainer-only panel guard, key validation - #670
Merged
Merged
Conversation
…-only panel guard, key validation) Follow-up hardening from a security/completeness audit of the BYOK feature (PRs #664/#665). The panel was already maintainer-only via the backend gate; these changes close the confirmed gaps. Backend: - Bound the BYOK provider fetch with AbortSignal.timeout (20s), mirroring the github/gittensor timeoutFetch convention, so a hung Anthropic/OpenAI connection can't stall the queue worker. Surface the failure reason (timeout/http_error/exception) in the ai_usage_events audit metadata for observability — never any key material. - Classify POST/DELETE /ai-key and PUT /ai-review as "expensive" in routeClassForPath (POST /ai-key runs PBKDF2 100k iters + an encrypted upsert). - Enforce the provider/key prefix check the schema comment already described: Anthropic keys must start with sk-ant-, OpenAI with sk- (and not sk-ant-), so a mismatched key is rejected with a 400 instead of failing silently later. Frontend: - Add an explicit maintainer/owner/operator role gate to MaintainerPanel via useSession, so the dashboard query and the BYOK form never mount for a non-maintainer (the backend 403 remains the source of truth) and a clean "Maintainer access required" message replaces the raw 403. - Mirror the provider/key prefix validation client-side for instant feedback. Tests: provider-fetch timeout signal + recorded failure reason, rate-limit classification, and provider/key mismatch rejection (400). Branch coverage 97%.
|
Note Gittensory Gate skippedPR closed before full evaluation. No late first comment was created.
💰 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
added
the
|
gittensory · advisory review Reviewed 8 changed file(s) — two independent AI reviewers. Suggested action: ✅ Safe to merge — both reviewers found no blocking issues. Reviewer A · Suggestions
Worth double-checking
Reviewer B · Suggestions
Worth double-checking
Before = production · After = this PR's preview deploy.
|
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
gittensory-ui | 5a29429 | Commit Preview URL Branch Preview URL |
Jun 14 2026, 01:41 AM |
Contributor
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
JSONbored
added a commit
that referenced
this pull request
Jun 14, 2026
…, per-record salt (#680) Second hardening round from the BYOK security audit (follow-up to #670). - #671 Audit the provider-key lifecycle. upsert/delete now emit an `ai_key_change` ai_usage_events row (status set|replace|delete, actor, display-only last4 — NEVER any key material), recorded as a non-"ok" status so it never counts toward the neuron budget. getRepositoryAiKeyStatus now surfaces createdBy + updatedAt so the dashboard can show who set the key and when. The DELETE route threads the session actor through. - #672 Stop counting BYOK advisory spend against the free Workers-AI daily neuron budget. The budget now meters only free calls (the consensus pair in block mode + the advisory leg when it is NOT BYOK); a BYOK advisory bills the maintainer's own account and still runs when the free budget is exhausted. - #677 Per-record PBKDF2 salt for the key-encryption envelope (v2). encryptSecret now generates a fresh random salt per record and stores it beside the IV; decryptSecret falls back to the legacy constant salt when no per-record salt is present, so existing v1 rows keep decrypting. Migration 0030 adds the nullable `salt` column. - #676 Clarify that repository_ai_keys.key_version is the crypto-envelope version (1 = legacy constant-salt, 2 = per-record salt), not a rotation counter. Tests: lifecycle audit trail (set→replace→delete, no key material, no-op delete); BYOK advisory runs with the free budget exhausted; v2 round-trip + v1 legacy decryption; non-numeric env clamp. Branch coverage 97%. Closes #671, #672, #676, #677. Part of #525.
JSONbored
added a commit
that referenced
this pull request
Jun 14, 2026
#681) Final round of the BYOK security-audit follow-ups (after #670, #680). - #673 Stand up a UI test runner (vitest + @testing-library/react + jsdom) in the gittensory-ui workspace and add the first component tests: the MaintainerPanel role gate (non-maintainer → "Maintainer access required", BYOK field never mounts; maintainer → dashboard) and AiReviewSettings (key field is write-only/password and never hydrates a stored key; provider/key mismatch is rejected client-side without a request; a valid key posts then clears). Wired `ui:test` into the CI "UI check" step. - #678 AI review panel UX + a11y: an initial loading state, an empty-state hint when no registered repos are detected, role="status" aria-live on the result banner, and aria-busy on the action buttons. - #675 Maintainer-facing BYOK documentation (docs/maintainer-byok-ai-review.md): how the advisory vs consensus layers work, that BYOK calls the provider directly and bills the maintainer (consensus always free), encrypted/write-only key handling, and dashboard vs .gittensory.yml config. Kept out of docs.github-app.tsx to avoid conflicting with the open onboarding-docs PR #648. Verified: ui:typecheck, ui:lint, ui:test (6 tests), ui:build, ui:openapi:check, actionlint, npm audit (0 vulns). Closes #673, #675, #678. Part of #525.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Follow-up hardening from a code-grounded security + completeness audit of the BYOK AI-review feature (#664 backend + #665 UI).
Audit headline (no action needed — already correct): the AI review / BYOK panel is shown only to server-verified maintainers. It's nested in
MaintainerPanel's data branch;GET /v1/app/maintainer-dashboard403sinsufficient_rolefor non-maintainers; the role is derived from real GitHub PR author-association / repo-ownership on installed repos (not self-asserted); and all 4 BYOK routes re-check per-repo maintainer access (401 unauth, 403 cross-repo). Key handling is sound: write-only, AES-256-GCM, random IV, isolated table, never logged/echoed/in public output.This PR closes the gaps the audit did confirm.
Changes
Backend
runProviderReviewissuedfetch()toapi.anthropic.com/api.openai.comwith noAbortController; a hung provider could stall the queue worker. Now bounded withAbortSignal.timeout(20s), matching the existinggithub/app.ts/gittensor/api.tstimeoutFetchconvention. The failure reason (timeout/http_error/exception) is recorded in theai_usage_eventsaudit metadata for observability — never any key material.POST/DELETE /ai-keyandPUT /ai-revieware nowexpensiveinrouteClassForPath(POST /ai-keyruns PBKDF2 100k iters + an encrypted D1 upsert). Reachable only by an authenticated repo-maintainer, but consistent with comparable heavy routes.repositoryAiKeySchemacomment already promised ansk-ant-…/sk-…check; now it's real — Anthropic keys must start withsk-ant-, OpenAI withsk-(and notsk-ant-). A mismatched key →400instead of failing silently at call time.Frontend
MaintainerPanelviauseSession: the dashboard query and the BYOK form never mount for a non-maintainer (the backend 403 remains the source of truth — this is defense-in-depth + a clean "Maintainer access required" message instead of a raw 403).Tests
byokFailure: "http_error"; abort recordsbyokFailure: "timeout"(and the audit row never contains the key).routeClassForPath→expensivefor/ai-keyand/ai-review.sk-prefix) →400.Verification
typecheck✅ ·test:coverage✅ (97.0% branch, 1672 tests) ·test:workers✅ ·git diff --check✅ui:typecheck✅ ·ui:lint✅ ·ui:build✅ ·ui:openapi:check✅part of #525