Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 78 additions & 2 deletions src/app/actions/issues.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ const mocks = vi.hoisted(() => ({
mockGetSession: vi.fn(),
mockServiceFrom: vi.fn(),
mockGetInstallOctokit: vi.fn(),
mockRateLimit: vi.fn(),
}));

vi.mock('@/lib/supabase/server', () => ({
Expand Down Expand Up @@ -47,16 +48,28 @@ vi.mock('@/lib/cache', () => ({
cacheDel: vi.fn().mockResolvedValue(undefined),
}));

import { getIssuesPage, getRepoOptions } from './issues';
vi.mock('@/lib/rate-limit', async (importOriginal) => {
const actual = await importOriginal<typeof import('@/lib/rate-limit')>();
return {
...actual,
rateLimit: mocks.mockRateLimit,
};
});

import { getIssuesPage, getRepoOptions, claimIssue } from './issues';

const createMockChain = (result: unknown) => {
const createMockChain = (result: unknown, singleResult: unknown = null) => {
const chain: Record<string, unknown> = {
select: vi.fn().mockReturnThis(),
eq: vi.fn().mockReturnThis(),
in: vi.fn().mockReturnThis(),
ilike: vi.fn().mockReturnThis(),
order: vi.fn().mockReturnThis(),
range: vi.fn().mockReturnThis(),
insert: vi.fn().mockReturnThis(),
update: vi.fn().mockReturnThis(),
single: vi.fn(() => Promise.resolve(singleResult)),
maybeSingle: vi.fn(() => Promise.resolve(singleResult)),
then: (resolve: (v: unknown) => void) => Promise.resolve(result).then(resolve),
};
return chain;
Expand Down Expand Up @@ -287,3 +300,66 @@ describe('getIssuesPage', () => {
});
});
});

describe('claimIssue', () => {
beforeEach(() => {
vi.clearAllMocks();
vi.unstubAllGlobals();
mocks.mockGetUser.mockResolvedValue({
data: { user: { id: 'user-1' } },
});
mocks.mockGetSession.mockResolvedValue({
data: { session: null },
});
mocks.mockGetInstallOctokit.mockResolvedValue({
repos: { get: vi.fn().mockResolvedValue({ data: { fork: false } }) },
});
mocks.mockRateLimit.mockResolvedValue({ ok: true, remaining: 19, resetAt: 0 });
});

it('creates a claim for an issue in a repo the user does not own', async () => {
mocks.mockServiceFrom
.mockReturnValueOnce(
createMockChain(
{},
{
data: { id: 1, difficulty: 'E', xp_reward: 50, repo_full_name: 'other/repo' },
error: null,
},
),
) // issues single
.mockReturnValueOnce(
createMockChain({}, { data: { github_handle: 'contributor' }, error: null }),
) // profiles maybeSingle
.mockReturnValueOnce(createMockChain({ data: [{ id: 10 }] })) // github_installations
.mockReturnValueOnce(
createMockChain({ data: [{ repo_full_name: 'other/repo', installation_id: 10 }] }),
) // installation_repositories
.mockReturnValueOnce(createMockChain({}, { data: null, error: null })) // recommendations lookup
.mockReturnValueOnce(createMockChain({}, { data: { id: 5 }, error: null })) // insert
.mockReturnValueOnce(createMockChain({})); // activity_log

const result = await claimIssue(1);

expect(result).toEqual({ ok: true, data: { recId: 5 } });
});

it('rejects claims on issues in a repository the user owns', async () => {
mocks.mockServiceFrom
.mockReturnValueOnce(
createMockChain(
{},
{
data: { id: 1, difficulty: 'E', xp_reward: 50, repo_full_name: 'owner/repo' },
error: null,
},
),
) // issues single
.mockReturnValueOnce(createMockChain({}, { data: { github_handle: 'owner' }, error: null })); // profiles maybeSingle

const result = await claimIssue(1);

expect(result.ok).toBe(false);
if (!result.ok) expect(result.error.code).toBe('forbidden');
});
});
13 changes: 13 additions & 0 deletions src/app/actions/issues.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { rateLimit, RATE_LIMIT_TIERS } from '@/lib/rate-limit';
import { cacheDel, cacheGet, cacheSet } from '@/lib/cache';
import { repoFilterPattern } from './issues-helpers';
import { getInstallOctokit } from '@/lib/github/app';
import { isSelfMerge } from '@/lib/xp/self-merge';

const PAGE_SIZE = 10;
const DIFFICULTY_VALUES = ['E', 'M', 'H'] as const;
Expand Down Expand Up @@ -318,6 +319,18 @@ export async function claimIssue(issueId: number): Promise<Result<{ recId: numbe
.single();
if (!issue) return err('not_found', 'issue not found');

// Anti-abuse (doc rule — self-actions on own repo don't count): reject
// claims on issues in a repository the user owns, so the
// claim -> merge -> recommended-XP loop can never be entered.
const { data: profile } = await service
.from('profiles')
.select('github_handle')
.eq('id', user.id)
.maybeSingle();
if (profile?.github_handle && isSelfMerge(issue.repo_full_name, profile.github_handle)) {
return err('forbidden', 'you cannot claim issues in a repository you own');
}

// Validate that the issue belongs to a repo the user has access to.
const repoOptsRes = await getRepoOptions();
if (!repoOptsRes.ok) return err(repoOptsRes.error.code, repoOptsRes.error.message);
Expand Down
47 changes: 46 additions & 1 deletion src/app/actions/recommendations.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,13 @@ describe('Recommendations Server Actions', () => {
describe('claimRecommendation', () => {
it('updates status to claimed and sets claimed_at, invalidating cache', async () => {
mocks.mockServiceFrom
.mockReturnValueOnce(createMockChain(null, { data: { github_handle: 'contributor' } })) // profile
.mockReturnValueOnce(
createMockChain(null, {
data: { id: 1, issue_id: 10, issues: { repo_full_name: 'other/repo' } },
error: null,
}),
) // rec + repo
.mockReturnValueOnce(createMockChain({ count: 0 })) // count claims
.mockReturnValueOnce(createMockChain(null, { data: { id: 1 }, error: null })) // update
.mockReturnValueOnce(createMockChain({})); // insert activity_log
Expand All @@ -273,6 +280,13 @@ describe('Recommendations Server Actions', () => {

it('returns already_claimed error if status is not open', async () => {
mocks.mockServiceFrom
.mockReturnValueOnce(createMockChain(null, { data: { github_handle: 'contributor' } })) // profile
.mockReturnValueOnce(
createMockChain(null, {
data: { id: 1, issue_id: 10, issues: { repo_full_name: 'other/repo' } },
error: null,
}),
) // rec + repo
.mockReturnValueOnce(createMockChain({ count: 0 })) // count claims
.mockReturnValueOnce(createMockChain(null, { data: null, error: null })); // update returns null row

Expand All @@ -285,7 +299,15 @@ describe('Recommendations Server Actions', () => {
});

it('returns claim_limit error if user has 3 or more claims', async () => {
mocks.mockServiceFrom.mockReturnValueOnce(createMockChain({ count: 3 })); // count claims
mocks.mockServiceFrom
.mockReturnValueOnce(createMockChain(null, { data: { github_handle: 'contributor' } })) // profile
.mockReturnValueOnce(
createMockChain(null, {
data: { id: 1, issue_id: 10, issues: { repo_full_name: 'other/repo' } },
error: null,
}),
) // rec + repo
.mockReturnValueOnce(createMockChain({ count: 3 })); // count claims

const result = await claimRecommendation(1);

Expand All @@ -304,8 +326,31 @@ describe('Recommendations Server Actions', () => {
if (!result.ok) expect(result.error.code).toBe('not_configured');
});

it('rejects claims on issues in a repository the user owns', async () => {
mocks.mockServiceFrom
.mockReturnValueOnce(createMockChain(null, { data: { github_handle: 'owner' } })) // profile
.mockReturnValueOnce(
createMockChain(null, {
data: { id: 1, issue_id: 10, issues: { repo_full_name: 'owner/repo' } },
error: null,
}),
); // rec + repo

const result = await claimRecommendation(1);

expect(result.ok).toBe(false);
if (!result.ok) expect(result.error.code).toBe('forbidden');
});

it('returns persist_failed error if update fails', async () => {
mocks.mockServiceFrom
.mockReturnValueOnce(createMockChain(null, { data: { github_handle: 'contributor' } })) // profile
.mockReturnValueOnce(
createMockChain(null, {
data: { id: 1, issue_id: 10, issues: { repo_full_name: 'other/repo' } },
error: null,
}),
) // rec + repo
.mockReturnValueOnce(createMockChain({ count: 0 })) // count claims
.mockReturnValueOnce(createMockChain(null, { data: null, error: new Error('DB Error') }));

Expand Down
29 changes: 29 additions & 0 deletions src/app/actions/recommendations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import { filterAndRank, type ScoredIssue } from '@/lib/pipeline/recommend';
import { capDifficulty, getAllowedDifficulties } from '@/lib/pipeline/difficulty';
import { getInstallationToken } from '@/lib/github/app';
import { listMaintainerInstalls, listMaintainerRepos } from '@/lib/maintainer/detect';
import { unwrapJoin } from '@/lib/supabase/inner-join';
import { isSelfMerge } from '@/lib/xp/self-merge';

/**
* Server actions for the recommendation lifecycle.
Expand Down Expand Up @@ -113,6 +115,33 @@ export async function claimRecommendation(recId: number): Promise<Result<{ id: n
});
if (!rateRes.ok) return err('rate_limited', 'slow down', true, rateRes.resetAt);

// Anti-abuse (doc rule — self-actions on own repo don't count): reject
// claims on issues in a repository the user owns, so the
// claim -> merge -> recommended-XP loop can never be entered.
const { data: profile } = await service
.from('profiles')
.select('github_handle')
.eq('id', user.id)
.maybeSingle();

const { data: recRow } = await service
.from('recommendations')
.select('id, issue_id, issues!inner(repo_full_name)')
.eq('id', recId)
.eq('user_id', user.id)
.maybeSingle();

const recIssue = unwrapJoin<{ repo_full_name?: string }>(
(recRow as unknown as { issues?: unknown }).issues,
);
if (
profile?.github_handle &&
recIssue?.repo_full_name &&
isSelfMerge(recIssue.repo_full_name, profile.github_handle)
) {
return err('forbidden', 'you cannot claim issues in a repository you own');
}

// Fast pre-check: reject early if the user is obviously at the limit.
// This is not authoritative — two concurrent requests can both pass it —
// but it avoids unnecessary write attempts under normal conditions.
Expand Down
22 changes: 20 additions & 2 deletions src/inngest/functions/process-pr-event.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ const prRun = processPrEvent as unknown as (ctx: {
}) => Promise<unknown>;

// Factory for a pull_request closed & merged event.
const ev = (prUrl: string, repo: string, number: number) => ({
const ev = (prUrl: string, repo: string, number: number, login = 'contributor') => ({
data: {
payload: {
action: 'closed',
Expand All @@ -43,7 +43,7 @@ const ev = (prUrl: string, repo: string, number: number) => ({
closed_at: '2026-01-01T00:00:00Z',
created_at: '2026-01-01T00:00:00Z',
updated_at: '2026-01-01T00:00:00Z',
user: { login: 'contributor' },
user: { login },
base: { repo: { full_name: repo } },
},
},
Expand Down Expand Up @@ -280,6 +280,24 @@ describe('processPrEvent - awardRecommendedMerge XP capping', () => {
}),
);
});

it('denies recommended merge XP when the PR author owns the repo (self_merge)', async () => {
const { activityLogMock } = setupMock({
id: 99,
user_id: 'owner-user',
difficulty: 'H',
xp_reward: 400,
status: 'claimed',
});

await prRun({
event: ev('https://github.com/owner/repo/pull/99', 'owner/repo', 99, 'owner'),
step,
});

expect(insertXpEvent).not.toHaveBeenCalled();
expect(activityLogMock.insert).not.toHaveBeenCalled();
});
});

describe('processPrEvent - linkPrToClaim issues relation array', () => {
Expand Down
21 changes: 14 additions & 7 deletions src/inngest/functions/process-pr-event.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { inngest } from '../client';
import { getServiceSupabase } from '@/lib/supabase/service';
import { insertXpEvent } from '@/lib/xp/events';
import { XP_SOURCE, xpForMerge, refIds, XP_REWARDS } from '@/lib/xp/sources';
import { isSelfMerge } from '@/lib/xp/self-merge';
import { cacheDelByPrefix } from '@/lib/cache';

import { buildPrRow, type IngestiblePr } from '@/lib/maintainer/pr-ingest';
Expand Down Expand Up @@ -350,6 +351,11 @@ export async function handleMerge(
const sb = getServiceSupabase();
if (!sb) throw new Error('service role missing');

// Anti-abuse (doc rule — self-actions on own repo don't count): a user can
// never earn merge XP for a PR they merged into a repository they own, on
// the recommended path or the unrecommended path.
if (isSelfMerge(repo, pr.user.login)) return { xpAwarded: false, reason: 'self_merge' };

// First try the linked rec.
const { data: rec } = await sb
.from('recommendations')
Expand Down Expand Up @@ -378,12 +384,7 @@ export async function handleMerge(
}
}

// Truly unrecommended. Anti-abuse: no XP when the author merges into
// their own repo (doc rule — self-actions on own repo don't count).
const repoOwner = repo.split('/')[0]?.toLowerCase();
const author = pr.user.login.toLowerCase();
if (repoOwner === author) return { xpAwarded: false, reason: 'self_merge' };

// Truly unrecommended: award baseline XP to the PR author.
const { data: profile } = await sb
.from('profiles')
.select('id')
Expand All @@ -409,7 +410,13 @@ async function awardRecommendedMerge(
rec: { id: number; user_id: string; difficulty: string; xp_reward: number | null },
repo: string,
pr: PrPayload['pull_request'],
): Promise<{ xpAwarded: boolean; recId: number }> {
): Promise<{ xpAwarded: boolean; recId: number; reason?: string }> {
// Same anti-abuse rule as the unrecommended path. handleMerge already guards
// this, but keep it here too so the award site can never drift.
if (isSelfMerge(repo, pr.user.login)) {
return { xpAwarded: false, recId: rec.id, reason: 'self_merge' };
}

const difficulty = rec.difficulty as 'E' | 'M' | 'H';
const tierCap =
XP_REWARDS.RECOMMENDED_MERGE[difficulty as keyof typeof XP_REWARDS.RECOMMENDED_MERGE] ??
Expand Down
19 changes: 19 additions & 0 deletions src/lib/xp/self-merge.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import { describe, it, expect } from 'vitest';
import { isSelfMerge } from './self-merge';

describe('isSelfMerge', () => {
it('matches the repo owner case-insensitively', () => {
expect(isSelfMerge('Owner/Repo', 'owner')).toBe(true);
expect(isSelfMerge('owner/repo', 'OWNER')).toBe(true);
});

it('does not match a non-owner author', () => {
expect(isSelfMerge('owner/repo', 'contributor')).toBe(false);
});

it('is false for missing inputs', () => {
expect(isSelfMerge('', 'owner')).toBe(false);
expect(isSelfMerge('owner/repo', '')).toBe(false);
expect(isSelfMerge('norepo', 'norepo')).toBe(false);
});
});
12 changes: 12 additions & 0 deletions src/lib/xp/self-merge.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/**
* Anti-abuse rule from the docs: "self-actions on own repo don't count".
* A user never earns merge XP (and never claims work) in a repository they
* own, regardless of whether the merge was recommended or not.
*
* Shared by handleMerge and the claim actions so the two layers cannot drift.
*/
export function isSelfMerge(repoFullName: string, githubLogin: string): boolean {
const slash = repoFullName.indexOf('/');
if (slash <= 0 || !githubLogin) return false;
return repoFullName.slice(0, slash).toLowerCase() === githubLogin.toLowerCase();
}
Loading