Repository navigation
Merged
Conversation
Sign in and install the LiveReview GitHub App in one GitHub round trip (/gh-login), connect installations to the user's org, and manage them from Git Providers. Backend - GitHub App auth: signed-state start/callback, ensure-cloud-user, webhook, installation-token minting/refresh, demo/disconnect/install-check endpoints (disconnect uninstalls via the App's own JWT; owners only, org scoped). - GitHub App connectors work across blast radius, scheduled/repo-sync workers, diff review, manual and webhook reviews (installation token resolution). - Reviews whose comments GitHub refuses (App not installed) complete with a "comments not posted" notice instead of failing; setup failures mark the review failed instead of leaving it stuck. - AI key validation has a 45s timeout with a clear message. Frontend - /gh-login landing page, get-started page, GitHub App card under Select Git Provider (accounts as panels with repositories, list/grid toggle, search), disconnect with loading state. - "App not installed" dialog on /reviews/new with install-in-new-tab and "I've installed it, continue review"; Install now on the Findings tab opens GitHub already on the repository owner. - Sign-in return replaces the saved session so flows can leave for GitHub without signing the user out. Docs: route guides, onboarding plan, storage/network status docs. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
- Go 1.26.9 -> 1.26.10 (go.mod, both Dockerfiles, docker-deps.env) for the crypto/x509 stdlib advisory GO-2026-6809. - ui/package-lock.json: follow-redirects 1.16.0 -> 1.16.1 and launch-editor 2.14.1 -> 2.14.2 (only those entries changed). - GitHub App auth: the state-cookie delete now sets Secure and SameSite like the cookie it expires; cookie findings that are intentional (UI-readable org and next-page cookies, Secure whenever HTTPS) are annotated with nosemgrep and the reason. - Annotate two semgrep false positives: the seeded math/rand in the test-only referencemix package, and the data: URI favicon in the blast-radius demo. gitleaks, govulncheck, OSV and semgrep now report nothing. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Cloud Free orgs were creator-only and the UI locked review creation, git provider connectors (and with them webhook reviews) and inviting members. - Server: drop the creator-only 403 from EnforceSubscriptionLimits; every member of a Free org can sign in and trigger reviews (quota status no longer limits Free to the creator). - Server: cap a Cloud Free org at 3 active members (license.FreePlanMaxMembers, same as self-hosted Community) when adding, bulk-inviting or re-activating a member; super admins are exempt. The org row is locked so concurrent invites can't both pass. - UI: remove the Free read-only locks on /reviews/new, the dashboard New Review button, onboarding "Configure AI", /git and connector delete. - UI: Invite User stops at 3 members on Cloud Free; the Free banner shows the member count; the always-true git-lrc Access column is removed. - UI: remove the client-side creator-only SubscriptionGuard and its modal, and the dead 'free' upsell in UserForm. - Route docs updated for the new Free behavior. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The fixed-layout table still had a width entry for the removed column, so the remaining columns covered only ~85% and left an empty strip on the right. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Inviting the email of a deactivated member failed with "already a member", and the UI has no other way to bring them back. The invite (single or bulk) now re-activates them with the chosen role, sends the invitation again and logs a "reactivated" audit entry. It still counts toward the Free plan member cap. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Implement GitHub App Onboarding and Connector IntegrationOverviewAdds native GitHub App authentication, onboarding workflows, and automatic repository synchronization. The update replaces personal access tokens with short-lived installation tokens and enforces single-organization installation tenancy. It also removes free-tier creator restrictions so all organization members can run reviews. Technical Highlights
Impact
|
![livereview-agent[bot]](https://avatars.githubusercontent.com/in/5380735?s=60&v=4){kind=small}
| -- migrate:up | ||
| -- One GitHub App installation can belong to exactly one LiveReview org. | ||
| CREATE UNIQUE INDEX IF NOT EXISTS uq_integration_tokens_github_app_installation | ||
| ON integration_tokens (provider_url, provider_app_id) |
There was a problem hiding this comment.
Severity: warning
PostgreSQL unique indexes treat NULLs as distinct by default. If provider_url is NULL, duplicate installations can be inserted.
Suggestions:
- Add
NULLS NOT DISTINCTto the index definition on PostgreSQL 15+. - Alternatively, guarantee
provider_urlis non-null with a default value like 'https://github.com'.
| These are exactly the fields `Cloud.tsx` reads (lines 70-85). Don't forward the full Parse user object. | ||
|
|
||
| *`GitHubAppWebhook`*: | ||
| 1. Read the body (`io.ReadAll`). |
There was a problem hiding this comment.
Severity: warning
Reading the raw webhook body without size limits enables memory exhaustion attacks.
Suggestions:
- Wrap the request body with
http.MaxBytesReaderorio.LimitReader. - Limit the allowed payload size to 5MB or 10MB.
| - Return 200. | ||
| 5. `installation_repositories` event: | ||
| - `added` → upsert those repos via `pullrequests.Store.UpsertRepository` and queue `QueueRepoPRSyncJob(…, true)`, the same as in `syncRepositoriesAndQueueBackfill`. | ||
| - `removed` → delete those `repositories` rows for this connector (new storage function `DeleteRepositoryByProviderID(connectorID, providerRepoID)` in `storage/providers/pullrequests`). |
There was a problem hiding this comment.
Severity: warning
Hard deleting repositories risks cascading deletion of historical reviews or causing foreign key violations.
Suggestions:
- Use soft deletion or an active status flag instead of hard DELETE.
- Preserve foreign key relationships for past reviews and pull requests.
| } | ||
|
|
||
| // Optional: enforce that token email matches requested email (defense-in-depth) | ||
| if strings.TrimSpace(claims.Email) != "" && !strings.EqualFold(claims.Email, req.Email) { |
There was a problem hiding this comment.
Severity: critical
Email verification is bypassed when claims.Email is empty, allowing arbitrary account impersonation with any valid JWT.
Suggestions:
- Require claims.Email to be non-empty and return an unauthorized error when absent.
- Derive the user email directly from verified token claims instead of the request body.
|
|
||
| // EnsureCloudUser ensures that a user and a personal organization exist, assigning super_admin role. | ||
| // Idempotent: if user/org/role mapping already exist, it returns success without changes. | ||
| func (h *AuthHandlers) EnsureCloudUser(c echo.Context) error { |
There was a problem hiding this comment.
Severity: critical
EnsureCloudUser was deleted here, but the new implementation file is missing from this changeset. The package will fail to compile.
Suggestions:
- Include internal/api/auth/ensure_cloud_user.go defining EnsureCloudUserCore, EnsureCloudUserInput, and EnsureCloudUser in this commit.
- Ensure test suites build and pass cleanly in CI before merging.
|
|
||
| return am.db.QueryRowContext(reqCtx, ` | ||
| SELECT obs.current_plan_code, COALESCE((o.created_by_user_id = ur.user_id), true) as is_creator | ||
| SELECT obs.current_plan_code |
There was a problem hiding this comment.
Severity: info
The orgs table join is redundant now that o.created_by_user_id is not selected. Join org_billing_state directly on ur.org_id.
Suggestions:
- Change to JOIN org_billing_state obs ON ur.org_id = obs.org_id and drop JOIN orgs o.
- Apply the same query cleanup to NewAuthMiddleware prepared statement on line 155.
| if n["repository"] != "HexmosTech/LiveReview" || n["can_install"] != true { | ||
| t.Fatalf("github with app configured: %+v", n) | ||
| } | ||
| if msg, _ := n["message"].(string); msg == "" || !contains(msg, "HexmosTech/LiveReview") { |
There was a problem hiding this comment.
Severity: warning
contains is used without importing strings. Use strings.Contains to avoid potential compilation errors.
Suggestions:
- Import
stringsand usestrings.Contains(msg, "HexmosTech/LiveReview"). - Verify test compilation with
go test ./internal/api.
| if liveFetch { | ||
| response["live_fetch"] = true | ||
| } | ||
| if _, ok := meta["comments_not_posted"]; ok { |
There was a problem hiding this comment.
Severity: critical
Checking key presence displays the notice even if comments_not_posted is false. Verify boolean value is true.
Suggestions:
- Check the boolean value using
if notPosted, ok := readBoolMeta(meta, "comments_not_posted"); ok && notPosted. - Prevent returning misleading unposted comment warnings to users when comments were successfully posted.
| err = networkgithub.DeleteInstallation(ctx, cfg.APIBaseURL(), appJWT, meta.InstallationID) | ||
| } | ||
| if err != nil { | ||
| log.Printf("[GitHubApp] disconnect connector %d (org %d): uninstalling installation %d failed: %v", connectorID, orgID, meta.InstallationID, err) |
There was a problem hiding this comment.
Severity: warning
If the App was uninstalled directly on GitHub, GitHub returns 404. Treating 404 as an error prevents local cleanup and strands the connector permanently.
Suggestions:
- Check if the GitHub error represents a 404 Not Found response.
- Treat 404 as success and proceed to clear connector flags and repository records.
| log.Printf("[GitHubApp] installation %d is bound to another org; skipping", inst.ID) | ||
| continue | ||
| } | ||
| if err != nil { |
There was a problem hiding this comment.
Severity: warning
Failing on one installation during non-explicit sign-in aborts the entire flow. Valid installations are skipped, and the user cannot log in.
Suggestions:
- When explicit is false, log the error and continue connecting remaining target installations.
- Only return an error if connected remains zero after inspecting all targets.
| MaxAge: 300, Secure: c.Scheme() == "https", SameSite: http.SameSiteLaxMode, | ||
| }) | ||
| } | ||
| return c.Redirect(http.StatusFound, "/?data="+url.QueryEscape(string(data))) |
There was a problem hiding this comment.
Severity: warning
Redirecting with authentication JWT and profile data in URL query strings exposes credentials in browser history, proxy logs, and Referer headers.
Suggestions:
- Pass authentication state through HttpOnly Secure cookies or a short-lived single-use exchange code.
- Avoid transmitting user JWTs directly in redirection URLs.
| } | ||
| } | ||
| go func() { | ||
| if err := s.autoWebhookInstaller.SyncAppConnector(context.Background(), int(connectorID)); err != nil { |
There was a problem hiding this comment.
Severity: warning
SyncAppConnector runs in a background goroutine with unconstrained context.Background(). Upstream stalls can cause goroutines to hang indefinitely.
Suggestions:
- Wrap context.Background() with context.WithTimeout.
- Set a reasonable deadline (e.g., 5 to 10 minutes) for background repository syncing.
| AccountType string `json:"account_type"` | ||
| RepositorySelection string `json:"repository_selection"` | ||
| } | ||
| _ = json.Unmarshal(rec.Metadata, &meta) |
There was a problem hiding this comment.
Severity: critical
Removed or suspended connectors are not checked, reporting inactive installations as connected (especially when light=1).
Suggestions:
- Add Removed and Suspended boolean fields to the meta struct unmarshaled from rec.Metadata.
- Check meta.Removed || meta.Suspended and continue early to avoid surfacing inactive installations.
| return nil | ||
| } | ||
| token, err := s.loadIntegrationTokenByID(id) | ||
| if err != nil || token.OrgID != orgID { |
There was a problem hiding this comment.
Severity: warning
Add a nil check for token before accessing OrgID. If loadIntegrationTokenByID returns nil without error, this panics.
Suggestions:
- Check
if err != nil || token == nil || token.OrgID != orgID {
| if err != nil || token.OrgID != orgID { | ||
| return nil | ||
| } | ||
| if !strings.Contains(strings.TrimRight(token.ProviderURL, "/"), baseURL) { |
There was a problem hiding this comment.
Severity: warning
Using strings.Contains on URLs allows domain confusion (e.g. evil.com containing target domain) and fails on trailing slashes.
Suggestions:
- Parse both token.ProviderURL and baseURL with url.Parse and compare normalized Host and Path values.
- Trim trailing slashes from both URLs before exact string comparison.
| // resolveGitHubAppToken replaces an App connector's cached token with a valid one (minting if it is near | ||
| // expiry). PAT connectors are left untouched. | ||
| func (s *Server) resolveGitHubAppToken(ctx context.Context, token *IntegrationToken) error { | ||
| if token.TokenType != storagegithub.TokenTypeGitHubApp { |
There was a problem hiding this comment.
Severity: warning
Guard against nil token to prevent a nil pointer dereference when accessing token.TokenType.
Suggestions:
- Add
if token == nil || token.TokenType != storagegithub.TokenTypeGitHubApp { return nil }
| } | ||
|
|
||
| // signGHState returns base64url(json) + "." + base64url(HMAC-SHA256(secret, "gh-app-state:" + base64url(json))). | ||
| func signGHState(s ghState, secret []byte) (string, error) { |
There was a problem hiding this comment.
Severity: warning
Validate secret length in signGHState. An empty secret creates states that verifyGHState will reject.
Suggestions:
- Return an error immediately if len(secret) == 0.
| if err != nil { | ||
| return nil, fmt.Errorf("database error: %w", err) | ||
| } | ||
| response.Projects = names |
There was a problem hiding this comment.
Severity: warning
Assigning nil names overwrites initialized slice. JSON marshaling produces null instead of an empty array, breaking clients.
Suggestions:
- Ensure names is initialized to an empty slice when empty before assigning to response.Projects.
- Keep response.Projects as an empty slice if ListRepositoryFullNamesByConnector returns nil.
- Sign-in: one installation failing to connect no longer aborts a plain GitHub sign-in; the others still connect. An explicit install still fails loudly, and if nothing connected because of errors the page says so. - Background repository sync now runs under a 15-minute timeout instead of an unbounded context. - The light "is a GitHub App connected?" check skips suspended installations, matching the full answer. - Repository access for App connectors returns [] rather than null when the installation has no repositories yet. - The preferred-connector lookup matches the provider host exactly instead of by substring. - Plan lookup in EnforceSubscriptionLimits joins org_billing_state directly (the orgs join was left over from the creator check). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Google retired gemini-2.5-flash / gemini-2.5-flash-lite on Vertex AI on 2026-10-20. The managed "LiveReview AI Model" tiers still pointed at them: the earlier bumps to 3.x matched provider_name 'gemini', but the rows are 'gemini-enterprise' (Vertex), so they never applied. - Migration: default -> gemini-3.8-flash, default_lite -> gemini-3.5-flash-lite, and a new gcp_location column set to 'us'. - Gemini 3.x is only served from global and the us/eu multi-regions. The Vertex SDK always dials "<location>-aiplatform.googleapis.com", which doesn't exist for those, so VertexEndpointOption points the client at aiplatform.googleapis.com / aiplatform.<loc>.rep.googleapis.com. Used by both Vertex clients (review provider and connector model), so BYOK Gemini Enterprise connectors set to us/eu/global work too. - The managed config now passes a GCP project (from the service-account key) and the tier's location; before, both were empty. Checked live against the managed service account: gemini-3.8-flash and gemini-3.5-flash-lite answer through the real Go client on "us". Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Managed AI (Settings → Managed AI, super admins on Cloud): - The "LiveReview AI Model" provider/model/key per tier (main, helper) was only settable with hand-written SQL in prod. A tier can now hold several configurations with exactly one active (partial unique index); super admins add one (e.g. OpenRouter deepseek/deepseek-v4-flash), test it and activate it. Activation and edits of the active one must pass a live test first; the previous config stays for switching back. - Keys are write-only (never returned or logged); writes and tests refuse API-key auth. New storage in storage/aiconnectors/managed_ai_store.go. - Managed configs now carry the provider's default base URL. Launch trial (/gh-login?src=launch-trial): - A new org created through that link is granted the LiveReview model on the Free plan (org_billing_state.free_managed_ai_granted_at) and gets the managed leader connector, so it can review without adding an AI key. Its own key still wins if it adds one; the Free plan's 30k LOC/month limit bounds the cost. Other src values and existing orgs are unchanged. - ConfirmPurchase now uses the same EnsureManagedConnector helper. - Get started shows "LiveReview AI is connected for you" with a link to use your own key instead. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…lected - loc_* plans treated the "LiveReview AI Model" connector (added on purchase) as the customer's own key, so its placeholder key was sent to the provider and reviews failed. Every paid plan now resolves that connector to the active managed configuration, like team_32usd did. - A paid Cloud org with no AI connector at all fell back to the LIVEREVIEW_HOSTED_* env vars, which aren't set in Cloud, so reviews failed. It now uses the managed model's main tier. A licensed self-hosted install keeps the env fallback. - The routing is a pure function (paidLeaderChoice) with unit tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Add and Save now make a test call first and only save when the model answers (Activate still re-tests before switching). - The Model field suggests IDs from the same catalog the AI Providers page uses (/aiconnectors/providers/:provider/models), with the provider's default as the placeholder; any ID can still be typed. Changing the provider clears the model. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The connector form's searchable model dropdown (catalog list, search box, "Custom model…") moves into a shared AIModelSelect component, used by both the AI Providers connector form and Settings → Managed AI, replacing the browser's native datalist there. Managed AI also pre-selects the provider's default model once the catalog loads, like the connector form. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- tests/mcp/requirements.txt: mcp 1.29.0 -> 1.30.0 (unbounded session accumulation in the StreamableHTTP transport). Its requirements are met by the existing pins. - scripts/uv.lock: werkzeug 3.1.8 -> 3.1.9 (safe_join() and Windows device names), via uv lock --upgrade-package werkzeug. The two follow-redirects alerts are already fixed on this branch (1.16.1). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
It held the first 17 characters of the real webhook secret (rest x'd out), which semgrep's generic-secret rule flags. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
lovestaco
changed the title
New advisories failed the osv-scanner CI job: - dompurify 3.4.16: GHSA-cg2g-xh64-v4q7 (critical, SAFE_FOR_TEMPLATES bypass), GHSA-mrv2-3pqr-j5jr (high, IN_PLACE hook), GHSA-c6gr-qvv4-5v3f (low, SMIL href guard). Fixed in 3.4.17. - ip-address 10.7.2: GHSA-rxp7-6hxx-x8q5 (moderate, quadratic zone-id matching). Fixed in 10.7.4. Both are pinned in ui/package.json overrides; only their two lock entries change (same dependencies, every dependent range satisfied).
2 of 7 tasks
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
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.
Summary
Everything for the Product Hunt launch flow: people sign in with GitHub, install the LiveReview GitHub App and get their first review without creating a PAT or bringing their own AI key. Along the way it opens up the Free plan, adds a super-admin page for the LiveReview-managed AI model, and moves that model off the retired Gemini 2.5.
1. "Continue with GitHub" with the LiveReview GitHub App (Cloud)
/gh-loginsigns the user in with GitHub. One GitHub screen installs the App and signs in. The Hexmos account is created through fw-parse (externalProviderSignup, providergithub_app; needs fw-parse MR !134), then the LiveReview user and org are created./get-started/github: first-run page showing the connected installations, repositories and open PRs, plus the next steps./git/github-app): connected GitHub accounts with their repositories, "Manage on GitHub", Disconnect, and "Add repositories or organization"./reviews/new: for a GitHub PR the App isn't installed on, a dialog offers to install it on that owner (in a new tab) or to run the review without posting comments. Reviews that couldn't post show a "comments weren't posted" notice.GET /api/v1/github-app/install-check.GITHUB_APP_*is configured.2. Free plan unlocks (Cloud)
3. Managed AI ("LiveReview AI Model")
gemini-3.8-flashandgemini-3.5-flash-liteon Vertex locationus. The earlier bumps never applied because they matched providergemini, while the rows aregemini-enterprise.<location>-aiplatform.googleapis.com, which doesn't exist forus,euorglobal, the only locations that serve Gemini 3.x. Vertex clients now point at the right host, so customers' own Gemini Enterprise connectors on those locations work too.AIModelSelectcomponent.loc_*plans treated the managed connector as the customer's own key and sent a placeholder key, so those reviews failed. Every paid plan now resolves it to the active managed config. A paid org with no AI connector at all also uses the managed model, instead of unset env vars.4. Launch trial:
/gh-login?src=launch-trialsrc, or an existing account, gets the normal flow.5. Security and dependencies
go.mod, both Dockerfiles,docker/docker-deps.env); follow-redirects 1.16.1; launch-editor 2.14.2; mcp 1.30.0 (tests/mcp); Werkzeug 3.1.9 (scripts/uv.lock). These clear the open Dependabot alerts onmaster..env.examplenow uses a placeholder for the webhook secret.Linked Issue
No linked issue; this is the Product Hunt launch work.
Validation
go build(all packages) andgo veton the changed packages. Tests pass forinternal/api(GitHub App, quota/plan, Managed AI),internal/api/auth,internal/api/users,internal/aiconnectors,internal/aiselection,internal/reviewandinternal/license. New tests cover:tsc --noEmitandmake check-status-doc.gemini-3.8-flashandgemini-3.5-flash-liteanswer through the real Go client onus.make security-govulncheck,-osv,-gitleaks,-semgrep,-dependabot,-gh-secret-scanning) are all clean on this branch./gh-loginflow, Free-plan member invites and the Managed AI page were tried on a dev tunnel.Checklist
SECURITY.mdand updated related documentation if needed.Notes For Reviewers
Behavior changes worth a look
internal/api/auth/middleware.go,internal/api/users/user_service.go). One existing Free org already has 4 members; it keeps them, since the cap only applies when members are added.internal/aiselection/aiselection.gonow routes all paid plans the same way.Migrations
20261020090000: unique GitHub App installation index;orgs.signup_path.20261022120000: managed tiers to Gemini 3.x, plusgcp_location.20261022130000: one active config per tier (the per-tier unique constraint is replaced by a partial unique index);label,updated_by_user_id;org_billing_state.free_managed_ai_granted_at.Before / after deploy
GITHUB_APP_*in the prod env, the App's webhook and callback URLs, and make the App public.deepseek/deepseek-v4-flashconfig in Settings → Managed AI.https://<prod>/gh-login?src=launch-trialas the Product Hunt link.🔍 LiveReview
⚙️ Customize risk-based approval policy for this repository or your organization.