Repository navigation
Feat/marble ink foundation - #8
Conversation
Two-stage verification funnel: deterministic file:line gate (pure, anti-fabrication invariant) then an LLM Critic that can only narrow — kept ⊆ gatePassed ⊆ candidates. Component is transcript-driven (SSE), live-only motion, surface+reasons human gate. Critic contract fixed now, live model call deferred to the connector session. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
phaseOf/progressOf/isAnimating map the authoritative ContainerState onto the console's visual phase so Agents console + Services summary derive one identical phase. Live-only motion: isAnimating true only while solving. 7 tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ore (spec step 2) - SynthStep gains optional needsAttention (Critic "wants a human look"). - ContainerStore interface (installationId-scoped) + in-memory sample impl that paces a working container's steps and replays finished ones instantly. - Sample IssueContainer fixtures (canonical shape) as the sample producer. - GET /api/containers/[id]/stream: SSE init/step/done, auth-gated, uniform 404. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Pure reducer + view derivation (ordering, kept/dropped/needs-attention counts, effective phase driven by the stream) with 9 tests; useSynthStream is a thin EventSource adapter over it, keyed per container by its consumer. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…, ledger (spec step 4) Pure synth-transcript-visual maps a SynthStep to marker/tone/spinner (spinner only on the live edge while animating → reduced-motion/finished renders final state, 7 tests). SynthProgress (phase bar), SynthAgentsRail (agents light on report), SynthTranscript (streamed keep/drop/⚑ lines), SynthLedger (verdict counts + raised human-gate card, disabled shell). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…spec step 5) SynthesizerConsole now streams a focused container's transcript over SSE (progress + live transcript + inline verdict counts + needs-attention + "approve in the Solution panel" pointer). No nested rail/approve button — AgentRail + PrPanel already frame it. Defaults to onboarding; "Watch a sample review" opts into a labelled Sample stream through the real path. 37 pipeline+synth tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…spec step 6) - SynthesizerSummary: condensed Services-side projection of the same stream (progress + specialists + verdict counts + Post-PR gate + deep-link). - /agents reads ?container= and focuses the console on it (the handoff). - Services IssuePanel renders the summary for the selected issue (sample container, labelled Sample, until Issue<->Container ids unify). Note: the react-hooks/set-state-in-effect error + IconCheck unused warning in services-workspace are pre-existing (test zero's replay effect), untouched here. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tep 7) verifyHybrid composes the deterministic file:line gate with a dependency- injected LLM Critic that may only uphold/drop/flag a gate-passed finding — so kept ⊆ gatePassed ⊆ candidates holds STRUCTURALLY (a model hallucination can drop a real finding, never fabricate a fake one). Fails open on Critic error (keep + flag, never a silent drop). 6 tests incl. a containment property test. Pipeline spec §4.5 amended to record the two-stage split. Live model call deferred to the connector session. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Per direction: the animated console must appear only when a REAL workflow is running, never fabricated data. Removed the "Watch a sample review" opt-in from the Agents console (empty state is now honest onboarding only; it streams solely a real deep-linked containerId) and removed the sample SynthesizerSummary from the Services IssuePanel. The components keep full streaming capability and mount on real IssueContainers at the connector step; sample fixtures remain for tests only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The six specialists + what each caught, from real commentsByCategory data (keyed by agent id == category). Calm Marble & Ink cards; honest "stands ready" before any review, "clear in its lane" at zero. Ties the Overview to the /agents workspace; no fabricated counts. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A "Copy agent prompt" action assembles the review's real verified findings (path:line · category · severity · body) into a paste-ready fix prompt for a coding agent. Prompt built server-side from real data; the client button only copies + shows a transient "Copied". Shown only when findings exist. Note: the page is already Marble & Ink (semantic tokens) — no dark-class re-skin was needed. Mermaid rendering of overallSummary stays a flagged future item (needs a deliberate mermaid dependency decision). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
reviewToContainer maps a stored review (getReviewDetail shape) into the container/transcript the Synthesizer console consumes — the honest bridge to real data. Review comments are all `kept` (verified upstream); the transcript narrates only real findings (no invented drops); terminal `posted` state, no live animation. Pure + 6 tests. Wires into a real ContainerStore next. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…from product path The container id is now a Kuma review id: the stream resolves the real tenancy-scoped review via getReviewContainer and emits its projected transcript. Removes the sample store from the live route entirely (a crafted sample id no longer streams fabricated data) — unknown/not-yours id → uniform 404 → empty console. Sample fixtures remain for tests only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Links a real review to /agents?container=<reviewId>, closing the real-data loop: review detail → the streaming Synthesizer console focused on that review (via getReviewContainer). A real navigation path, no fabricated data. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…c-stage Corrects §5.3 to defer to the real Critic (packages/agents/critic.py, in main): cross-tier opus<->sonnet, binary keep/drop, no SYNTH_CRITIC_MODEL. Reframes the dashboard's role as CONSUME (display backend verdicts + dropped_count/critic_dropped_count) not re-run. Records the open cross-lane follow-up: surface the drop counts once the DB Review columns land. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…synthesizer bug-zap animation, page restructuring - Hero agent demo (HeroAgentGraph): auto-plays Synthesizer thinking animation on page load, starts with thinking state visible - Services demo (ServicesWorkspace): auto-plays triage timeline when scrolled into view via useInView with hydration guard, resets on scroll-out - Specialist roster (SpecialistRoster): auto-cycles through agents every 5s when in view, pauses on manual click - Synthesizer visual (SynthesizerVisual): bug-zap animation with correct colors (red bug border, green PR border) - Landing page restructured: ConnectorStrip above HowItWorks, FeatureBlocks with Specialists/Synthesizer, ServicesPreview below - Fixed hydration mismatches with Math.round() on animation geometry - Specialist roster icon backgrounds use agent color tint instead of harsh black/white - Removed duplicate AgentsPreview section
…ation # Conflicts: # packages/dashboard/src/app/page.tsx
…onfigurable The install links hardcoded github.com/apps/arete-ai-code-review, which 404s (no such app). Now derived from GITHUB_APP_SLUG (default kumaservices) + /installations/new, so it resolves and is one env var to change per env. EmptyState test updated. 2/2 green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
When a repo is connected (installations present) the empty states now say 'Connected — open a pull request' with the next step, instead of a stale 'Connect a source/repository' CTA. Distinguishes connected-awaiting-review from not-connected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
When a repo is connected but no review has run, the console onboarding shows 'Connected · awaiting your first pull request' instead of the 'Connect a repository' CTA. Threaded connected (hasAccess) page->workspace->console.
When the repo is connected, the Services rail CTA points to the real next
step ('Connect a telemetry source') rather than a generic 'Connect your
services' — services are telemetry-gated (Sentry etc.), distinct from the
repo connection.
…t from main) /agents center now shows the selected specialist: its real findings, an agent-profile preview (description + what it inspects) when it has none, and a live chat composer proxied to the FastAPI /chat (honestly gated). Rail select/configure decoupled (gear button). Adds getAgentActivity query and /api/agents/[id]/chat route. /services center gains a Connect-a-repository CTA. Marble synth-* code retained (no longer the /agents center). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…rift safe) Using include fetched every ReviewComment scalar, including noiseState which is absent on databases predating that migration, causing a runtime PrismaClientKnownRequestError on /agents. Select the exact columns instead. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Both login paths hardcoded user.id="dummy-user-id", collapsing every user into one identity so a second user inherited the first's installations/ reviews/billing (a live cross-tenant read leak). Now: - Credentials authorize() -> verifyCredentials() returns the real User.id - Google signIn() -> upsertGoogleUser() upserts a real User row + uses its id Both helpers were already imported-but-unused. Adds a no-collision tenancy test (user A never inherits user B's installations) + unlinked -> []. 17 auth/users tests green, tsc clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The authenticated (embedded) /services center now renders the streaming SynthesizerConsole (reused from the orphaned /agents center) — onboarding now, live once an Issue<->Container backs the selected issue. The framed marketing preview keeps its scripted per-issue replay. Makes /services the Synthesizer's canonical home. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Backend (getReviewContainer + /api/containers/[id]/stream SSE) already exists. Wire it through: /services?container=<reviewId> streams that real review in the center Synthesizer; reroute the review-page watch link from /agents to /services (the synth moved there). Fixes the deep-link regression from the per-agent-chat port. Live once the ReviewComment noise migration is applied. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Strix Security Review3 open security findings on this PR:
Review summaryReviewed all security-relevant changes in the PR, including the new dashboard API routes, tenancy/auth updates, Synthesizer projection path, and the webhook backfill workflow. Confirmed two medium-severity issues in the newly added code: Updated for Reviewed by Strix |
| import { NextResponse, type NextRequest } from "next/server"; | ||
| import { auth } from "@/lib/auth"; | ||
| import { AGENTS } from "@/components/dashboard/agents/agent-catalog"; | ||
| import { sendAgentChat } from "@/lib/agent-chat"; |
There was a problem hiding this comment.
🔵 Dashboard agent chat lacks installation entitlement and billing enforcement
Severity: MEDIUM · CWE-862
The new dashboard chat route authorizes any logged-in dashboard account and forwards prompts directly to the Python /chat model without checking whether the caller administers any connected installation or whether that installation is still entitled to consume billable AI features. This bypasses the existing billing gate already enforced for the webhook review-comment chat flow and allows self-registered accounts or billing-blocked tenants to consume model resources through the dashboard.
| import { NextResponse, type NextRequest } from "next/server"; | |
| import { auth } from "@/lib/auth"; | |
| import { AGENTS } from "@/components/dashboard/agents/agent-catalog"; | |
| import { sendAgentChat } from "@/lib/agent-chat"; | |
| import { NextResponse, type NextRequest } from "next/server"; | |
| import { auth } from "@/lib/auth"; | |
| import { db } from "@/lib/db"; | |
| import { AGENTS } from "@/components/dashboard/agents/agent-catalog"; | |
| import { sendAgentChat } from "@/lib/agent-chat"; | |
| import { FREE_TIER_REVIEW_LIMIT } from "@/lib/queries"; |
Prompt to fix with AI
This is a security vulnerability found during a code review.
Vulnerability: Dashboard agent chat lacks installation entitlement and billing enforcement
Severity: MEDIUM
CWE: CWE-862
The new dashboard chat route authorizes any logged-in dashboard account and forwards prompts directly to the Python `/chat` model without checking whether the caller administers any connected installation or whether that installation is still entitled to consume billable AI features. This bypasses the existing billing gate already enforced for the webhook review-comment chat flow and allows self-registered accounts or billing-blocked tenants to consume model resources through the dashboard.
Location: packages/dashboard/src/app/api/agents/[id]/chat/route.ts:1-4
Context: Add installation and billing imports to the dashboard chat route
```
// Before:
import { NextResponse, type NextRequest } from "next/server";
import { auth } from "@/lib/auth";
import { AGENTS } from "@/components/dashboard/agents/agent-catalog";
import { sendAgentChat } from "@/lib/agent-chat";
// After:
import { NextResponse, type NextRequest } from "next/server";
import { auth } from "@/lib/auth";
import { db } from "@/lib/db";
import { AGENTS } from "@/components/dashboard/agents/agent-catalog";
import { sendAgentChat } from "@/lib/agent-chat";
import { FREE_TIER_REVIEW_LIMIT } from "@/lib/queries";
```
Location: packages/dashboard/src/app/api/agents/[id]/chat/route.ts:20-39
Context: Reject users with no authorized installation and enforce the chat billing gate before proxying to `/chat`
```
// Before:
const session = await auth();
if (!session?.user) {
return NextResponse.json({ error: "Unauthorized" }, { status: 401 });
}
const { id } = await ctx.params;
const agent = AGENTS.find((a) => a.id === id);
if (!agent) {
return NextResponse.json({ error: "Unknown agent" }, { status: 400 });
}
const body = await req.json().catch(() => ({}));
const message = typeof body?.message === "string" ? body.message.trim() : "";
if (!message) {
return NextResponse.json({ error: "Empty message" }, { status: 400 });
}
try {
const reply = await sendAgentChat({ agent, message });
return NextResponse.json({ reply });
// After:
const session = await auth();
if (!session?.user) {
return NextResponse.json({ error: "Unauthorized" }, { status: 401 });
}
const installationIds = (session.installations ?? []).map((installation) => installation.id);
if (installationIds.length === 0) {
return NextResponse.json(
{ error: "Connect an authorized installation before using agent chat." },
{ status: 403 },
);
}
const entitledInstallation = await db.installation.findFirst({
where: {
id: { in: installationIds },
OR: [
{ subscriptionStatus: "active" },
{
AND: [
{ subscriptionStatus: { notIn: ["canceled", "past_due"] } },
{ usageCount: { lt: FREE_TIER_REVIEW_LIMIT } },
],
},
],
},
select: { id: true },
});
if (!entitledInstallation) {
return NextResponse.json(
{ error: "Agent chat is unavailable for this installation's current plan." },
{ status: 403 },
);
}
const { id } = await ctx.params;
const agent = AGENTS.find((a) => a.id === id);
if (!agent) {
return NextResponse.json({ error: "Unknown agent" }, { status: 400 });
}
const body = await req.json().catch(() => ({}));
const message = typeof body?.message === "string" ? body.message.trim() : "";
if (!message) {
return NextResponse.json({ error: "Empty message" }, { status: 400 });
}
try {
const reply = await sendAgentChat({ agent, message });
return NextResponse.json({ reply });
```
How to fix:
Require a repository installation entitlement before proxying dashboard chat requests. The route should reject users with no authorized installations, then enforce the same billing policy already used by the webhook chat flow before calling the Python service. If chat is meant to be scoped to a specific installation, require an `installationId` parameter, verify it belongs to `session.installations`, and evaluate the billing gate against that installation; the UI can then disable the composer when the server would deny access.
Please fix this vulnerability. If you propose a fix, make it concise and minimal.React 👍 / 👎 to tune Strix for this repo. A repo collaborator (or the PR author) can resolve this thread to dismiss the finding.
| const session = await auth(); | ||
| if (!session?.user) { | ||
| return NextResponse.json({ error: "Unauthorized" }, { status: 401 }); | ||
| } | ||
|
|
||
| const { id } = await ctx.params; | ||
| const agent = AGENTS.find((a) => a.id === id); | ||
| if (!agent) { | ||
| return NextResponse.json({ error: "Unknown agent" }, { status: 400 }); | ||
| } | ||
|
|
||
| const body = await req.json().catch(() => ({})); | ||
| const message = typeof body?.message === "string" ? body.message.trim() : ""; | ||
| if (!message) { | ||
| return NextResponse.json({ error: "Empty message" }, { status: 400 }); | ||
| } | ||
|
|
||
| try { | ||
| const reply = await sendAgentChat({ agent, message }); | ||
| return NextResponse.json({ reply }); |
There was a problem hiding this comment.
Reject users with no authorized installation and enforce the chat billing gate before proxying to /chat
| const session = await auth(); | |
| if (!session?.user) { | |
| return NextResponse.json({ error: "Unauthorized" }, { status: 401 }); | |
| } | |
| const { id } = await ctx.params; | |
| const agent = AGENTS.find((a) => a.id === id); | |
| if (!agent) { | |
| return NextResponse.json({ error: "Unknown agent" }, { status: 400 }); | |
| } | |
| const body = await req.json().catch(() => ({})); | |
| const message = typeof body?.message === "string" ? body.message.trim() : ""; | |
| if (!message) { | |
| return NextResponse.json({ error: "Empty message" }, { status: 400 }); | |
| } | |
| try { | |
| const reply = await sendAgentChat({ agent, message }); | |
| return NextResponse.json({ reply }); | |
| const session = await auth(); | |
| if (!session?.user) { | |
| return NextResponse.json({ error: "Unauthorized" }, { status: 401 }); | |
| } | |
| const installationIds = (session.installations ?? []).map((installation) => installation.id); | |
| if (installationIds.length === 0) { | |
| return NextResponse.json( | |
| { error: "Connect an authorized installation before using agent chat." }, | |
| { status: 403 }, | |
| ); | |
| } | |
| const entitledInstallation = await db.installation.findFirst({ | |
| where: { | |
| id: { in: installationIds }, | |
| OR: [ | |
| { subscriptionStatus: "active" }, | |
| { | |
| AND: [ | |
| { subscriptionStatus: { notIn: ["canceled", "past_due"] } }, | |
| { usageCount: { lt: FREE_TIER_REVIEW_LIMIT } }, | |
| ], | |
| }, | |
| ], | |
| }, | |
| select: { id: true }, | |
| }); | |
| if (!entitledInstallation) { | |
| return NextResponse.json( | |
| { error: "Agent chat is unavailable for this installation's current plan." }, | |
| { status: 403 }, | |
| ); | |
| } | |
| const { id } = await ctx.params; | |
| const agent = AGENTS.find((a) => a.id === id); | |
| if (!agent) { | |
| return NextResponse.json({ error: "Unknown agent" }, { status: 400 }); | |
| } | |
| const body = await req.json().catch(() => ({})); | |
| const message = typeof body?.message === "string" ? body.message.trim() : ""; | |
| if (!message) { | |
| return NextResponse.json({ error: "Empty message" }, { status: 400 }); | |
| } | |
| try { | |
| const reply = await sendAgentChat({ agent, message }); | |
| return NextResponse.json({ reply }); |
| import type { Octokit } from '@octokit/core' | ||
| import { enqueueReviewJob } from './queue.js' |
There was a problem hiding this comment.
🔵 Duplicate PR review posting in GitHub installation backfill flow
Severity: MEDIUM
The new GitHub backfill flow introduced in this PR enqueues review jobs for every open pull request during installation and installation_repositories events without checking whether the same (repository, prNumber, headSha) was already reviewed. Because the queued worker publishes the PR review and check run before persistence rejects duplicates, a redelivered installation event or repeated repository-add event causes duplicate review comments, duplicate check runs, and repeated review-pipeline spend for already-reviewed pull requests.
| import type { Octokit } from '@octokit/core' | |
| import { enqueueReviewJob } from './queue.js' | |
| import type { Octokit } from '@octokit/core' | |
| import { reviewExists } from './persistence.js' | |
| import { enqueueReviewJob } from './queue.js' |
Prompt to fix with AI
This is a security vulnerability found during a code review.
Vulnerability: Duplicate PR review posting in GitHub installation backfill flow
Severity: MEDIUM
The new GitHub backfill flow introduced in this PR enqueues review jobs for every open pull request during `installation` and `installation_repositories` events without checking whether the same `(repository, prNumber, headSha)` was already reviewed. Because the queued worker publishes the PR review and check run before persistence rejects duplicates, a redelivered installation event or repeated repository-add event causes duplicate review comments, duplicate check runs, and repeated review-pipeline spend for already-reviewed pull requests.
Location: packages/webhook/src/backfill.ts:1-2
Context: Add early idempotency import
```
// Before:
import type { Octokit } from '@octokit/core'
import { enqueueReviewJob } from './queue.js'
// After:
import type { Octokit } from '@octokit/core'
import { reviewExists } from './persistence.js'
import { enqueueReviewJob } from './queue.js'
```
Location: packages/webhook/src/backfill.ts:82-95
Context: Skip already-reviewed PRs during backfill
```
// Before:
const changedFiles = pr.changed_files ?? 0
const lane = changedFiles > 50 ? 'heavy' : 'fast'
await enqueueReviewJob({
provider: 'github',
kind: 'pull_request',
owner,
repo: name,
repositoryExternalId: repo.id,
fullName: repo.full_name,
installationId,
prNumber: pr.number,
headSha: pr.head.sha,
}, lane)
// After:
const changedFiles = pr.changed_files ?? 0
const lane = changedFiles > 50 ? 'heavy' : 'fast'
const alreadyReviewed = await reviewExists({
provider: 'github',
repositoryExternalId: repo.id,
prNumber: pr.number,
headSha: pr.head.sha,
})
if (alreadyReviewed) {
console.log(
`[backfill] Review already exists for ${repo.full_name}#${pr.number} @ ${pr.head.sha} — skipping duplicate backfill job`
)
continue
}
await enqueueReviewJob({
provider: 'github',
kind: 'pull_request',
owner,
repo: name,
repositoryExternalId: repo.id,
fullName: repo.full_name,
installationId,
prNumber: pr.number,
headSha: pr.head.sha,
}, lane)
```
Location: packages/webhook/src/server.ts:70-78
Context: Installation event reaches vulnerable backfill path
```
try {
const { getInstallationOctokit } = await import('./github-auth.js')
const octokit = await getInstallationOctokit(app, payload.installation.id)
const { backfillInstallationPRs } = await import('./backfill.js')
// `repositories` is only present on some installation actions (e.g.
// 'created'); default to [] so e.g. 'unsuspend' deliveries without it
// are a no-op rather than a crash.
const repos = (payload as any).repositories ?? []
await backfillInstallationPRs(octokit as any, payload.installation.id, repos)
```
Location: packages/webhook/src/server.ts:87-93
Context: Repository-add event reaches vulnerable backfill path
```
app.webhooks.on('installation_repositories', async ({ payload }) => {
if (payload.action !== 'added') return
try {
const { getInstallationOctokit } = await import('./github-auth.js')
const octokit = await getInstallationOctokit(app, payload.installation.id)
const { backfillInstallationPRs } = await import('./backfill.js')
await backfillInstallationPRs(octokit as any, payload.installation.id, payload.repositories_added as any)
```
How to fix:
Before enqueueing each backfilled pull request, perform the same `(provider, repository external ID, PR number, head SHA)` idempotency check used by the normal `pull_request` webhook path. Skip enqueueing when a matching review already exists so duplicate installation deliveries and repeated repository-add events do not trigger another worker run.
Please fix this vulnerability. If you propose a fix, make it concise and minimal.React 👍 / 👎 to tune Strix for this repo. A repo collaborator (or the PR author) can resolve this thread to dismiss the finding.
| const changedFiles = pr.changed_files ?? 0 | ||
| const lane = changedFiles > 50 ? 'heavy' : 'fast' | ||
|
|
||
| await enqueueReviewJob({ | ||
| provider: 'github', | ||
| kind: 'pull_request', | ||
| owner, | ||
| repo: name, | ||
| repositoryExternalId: repo.id, | ||
| fullName: repo.full_name, | ||
| installationId, | ||
| prNumber: pr.number, | ||
| headSha: pr.head.sha, | ||
| }, lane) |
There was a problem hiding this comment.
Skip already-reviewed PRs during backfill
| const changedFiles = pr.changed_files ?? 0 | |
| const lane = changedFiles > 50 ? 'heavy' : 'fast' | |
| await enqueueReviewJob({ | |
| provider: 'github', | |
| kind: 'pull_request', | |
| owner, | |
| repo: name, | |
| repositoryExternalId: repo.id, | |
| fullName: repo.full_name, | |
| installationId, | |
| prNumber: pr.number, | |
| headSha: pr.head.sha, | |
| }, lane) | |
| const changedFiles = pr.changed_files ?? 0 | |
| const lane = changedFiles > 50 ? 'heavy' : 'fast' | |
| const alreadyReviewed = await reviewExists({ | |
| provider: 'github', | |
| repositoryExternalId: repo.id, | |
| prNumber: pr.number, | |
| headSha: pr.head.sha, | |
| }) | |
| if (alreadyReviewed) { | |
| console.log( | |
| `[backfill] Review already exists for ${repo.full_name}#${pr.number} @ ${pr.head.sha} — skipping duplicate backfill job` | |
| ) | |
| continue | |
| } | |
| await enqueueReviewJob({ | |
| provider: 'github', | |
| kind: 'pull_request', | |
| owner, | |
| repo: name, | |
| repositoryExternalId: repo.id, | |
| fullName: repo.full_name, | |
| installationId, | |
| prNumber: pr.number, | |
| headSha: pr.head.sha, | |
| }, lane) |
| try { | ||
| const { getInstallationOctokit } = await import('./github-auth.js') | ||
| const octokit = await getInstallationOctokit(app, payload.installation.id) | ||
| const { backfillInstallationPRs } = await import('./backfill.js') | ||
| // `repositories` is only present on some installation actions (e.g. | ||
| // 'created'); default to [] so e.g. 'unsuspend' deliveries without it | ||
| // are a no-op rather than a crash. | ||
| const repos = (payload as any).repositories ?? [] | ||
| await backfillInstallationPRs(octokit as any, payload.installation.id, repos) |
There was a problem hiding this comment.
Installation event reaches vulnerable backfill path
| app.webhooks.on('installation_repositories', async ({ payload }) => { | ||
| if (payload.action !== 'added') return | ||
| try { | ||
| const { getInstallationOctokit } = await import('./github-auth.js') | ||
| const octokit = await getInstallationOctokit(app, payload.installation.id) | ||
| const { backfillInstallationPRs } = await import('./backfill.js') | ||
| await backfillInstallationPRs(octokit as any, payload.installation.id, payload.repositories_added as any) |
There was a problem hiding this comment.
Repository-add event reaches vulnerable backfill path
| import type { Octokit } from '@octokit/core' | ||
| import { enqueueReviewJob } from './queue.js' |
There was a problem hiding this comment.
🔵 Billing gate bypass in GitHub installation backfill flow
Severity: MEDIUM · CWE-840
The PR introduces GitHub installation and installation_repositories backfill handlers that enqueue review jobs for every existing open pull request without applying the billing gate used by the normal pull_request and pull_request_review_comment flows. Because the worker posts the review before persistence updates usage and does not re-check billing, a customer with an inactive subscription or an exhausted free tier can reinstall the app or add repositories and still receive LLM-backed reviews.
| import type { Octokit } from '@octokit/core' | |
| import { enqueueReviewJob } from './queue.js' | |
| import type { Octokit } from '@octokit/core' | |
| import { prisma } from './db.js' | |
| import { evaluateBillingGate } from './billing.js' | |
| import { enqueueReviewJob } from './queue.js' |
Prompt to fix with AI
This is a security vulnerability found during a code review.
Vulnerability: Billing gate bypass in GitHub installation backfill flow
Severity: MEDIUM
CWE: CWE-840
The PR introduces GitHub `installation` and `installation_repositories` backfill handlers that enqueue review jobs for every existing open pull request without applying the billing gate used by the normal `pull_request` and `pull_request_review_comment` flows. Because the worker posts the review before persistence updates usage and does not re-check billing, a customer with an inactive subscription or an exhausted free tier can reinstall the app or add repositories and still receive LLM-backed reviews.
Location: packages/webhook/src/backfill.ts:1-2
Context: Add billing dependencies to the backfill module
```
// Before:
import type { Octokit } from '@octokit/core'
import { enqueueReviewJob } from './queue.js'
// After:
import type { Octokit } from '@octokit/core'
import { prisma } from './db.js'
import { evaluateBillingGate } from './billing.js'
import { enqueueReviewJob } from './queue.js'
```
Location: packages/webhook/src/backfill.ts:58-62
Context: Block installation/repository backfill when billing is not allowed
```
// Before:
export async function backfillInstallationPRs(
octokit: Octokit,
installationId: number,
repos: BackfillRepo[]
): Promise<void> {
// After:
export async function backfillInstallationPRs(
octokit: Octokit,
installationId: number,
repos: BackfillRepo[]
): Promise<void> {
const installation = await prisma.installation.findUnique({
where: { provider_externalId: { provider: 'github', externalId: installationId } }
})
const gate = evaluateBillingGate(installation)
if (!gate.allowed) {
console.log(
`[backfill] Skipping PR backfill for installation ${installationId} (${gate.reason}). ` +
`Status: ${installation?.subscriptionStatus}, usage: ${installation?.usageCount}`
)
return
}
```
How to fix:
Apply the same billing decision before any backfill job is enqueued. The simplest fix is to load the installation record inside `backfillInstallationPRs`, evaluate it with the existing billing helper, and return early when the installation is blocked. As defense in depth, keep billing enforcement close to job execution as well so any future alternate enqueue path cannot bypass the check.
Please fix this vulnerability. If you propose a fix, make it concise and minimal.React 👍 / 👎 to tune Strix for this repo. A repo collaborator (or the PR author) can resolve this thread to dismiss the finding.
| export async function backfillInstallationPRs( | ||
| octokit: Octokit, | ||
| installationId: number, | ||
| repos: BackfillRepo[] | ||
| ): Promise<void> { |
There was a problem hiding this comment.
Block installation/repository backfill when billing is not allowed
| export async function backfillInstallationPRs( | |
| octokit: Octokit, | |
| installationId: number, | |
| repos: BackfillRepo[] | |
| ): Promise<void> { | |
| export async function backfillInstallationPRs( | |
| octokit: Octokit, | |
| installationId: number, | |
| repos: BackfillRepo[] | |
| ): Promise<void> { | |
| const installation = await prisma.installation.findUnique({ | |
| where: { provider_externalId: { provider: 'github', externalId: installationId } } | |
| }) | |
| const gate = evaluateBillingGate(installation) | |
| if (!gate.allowed) { | |
| console.log( | |
| `[backfill] Skipping PR backfill for installation ${installationId} (${gate.reason}). ` + | |
| `Status: ${installation?.subscriptionStatus}, usage: ${installation?.usageCount}` | |
| ) | |
| return | |
| } |
…ing out new main logic for review
No description provided.