Skip to content

fix(maintainer): scope flagged-accounts query at database level (closes #755)#810

Open
arcgod-design wants to merge 1 commit into
Coder-s-OG-s:mainfrom
arcgod-design:feat/issue-755-scoped-flagged-accounts
Open

fix(maintainer): scope flagged-accounts query at database level (closes #755)#810
arcgod-design wants to merge 1 commit into
Coder-s-OG-s:mainfrom
arcgod-design:feat/issue-755-scoped-flagged-accounts

Conversation

@arcgod-design

Copy link
Copy Markdown
Contributor

Summary

Closes #755 — replaces the unbounded global load + client-side filter pattern in getFlaggedAccounts with query-level scoping.

Before

src/app/actions/maintainer/flagged-accounts.ts fetched up to 100 rows from flagged_accounts across all organizations and then filtered the result in JavaScript:

await service.from('flagged_accounts').select('...').eq('status','open').limit(100);
// ...later...
const allowedFlags = flags.filter((flag) => {
  // filter by evidence.items[].repo contained in maintainer's repos
});

This leaked global row counts via timing side-channels and wasted database resources on every maintainer action call.

After

The user_ids that have activity in the maintainer's repos are resolved first from pull_requests and recommendations in parallel via Promise.all. Only those user_ids are then passed to the flagged_accounts query as an .in('user_id', userIdsFilter) filter — so the database scope is now equivalent to the maintainer's authorized surface, matching closePullRequest, requestChanges, getContributorsList, etc. that already scope at query level.

If activeUserIds is empty, the function short-circuits before ever touching flagged_accounts, eliminating the entire data-leak surface for maintainers with no in-scope user activity.

The JavaScript evidence.items[].repo filter is retained as defense-in-depth (JSONB containment checks are not portable across Supabase client versions and the column has no GIN index guaranteed).

Tests

src/app/actions/maintainer.test.ts — added 5 new regression tests for getFlaggedAccounts scoping:

  1. scopes flagged_accounts query at the database level to active user ids — asserts the .in('user_id', ...) call was made with only users that have activity in the maintainer's repos and that user-inactive is not present.
  2. returns empty array without querying flagged_accounts when no users have activity — guards the data-leak short-circuit: when zero PRs/recommendations exist for the maintainer's repos, the global flagged_accounts table is never queried at all (flaggedAccountsCallCount === 0).
  3. propagates query error when pull_requests activity lookup fails — error code query_failed.
  4. propagates query error when recommendations activity lookup fails — error code query_failed.
  5. still drops flags whose evidence.items repo is outside maintainer scope — defense-in-depth post-filter still works.

The 3 existing scoping tests continue to pass unchanged. Total: 117 tests pass in src/app/actions/maintainer.test.ts.

Validation

  • vitest run src/app/actions/maintainer.test.ts — 117 passed
  • eslint src/app/actions/maintainer/flagged-accounts.ts src/app/actions/maintainer.test.ts — clean
  • prettier --check — All matched files use Prettier code style
  • tsc --noEmit -p tsconfig.json — no errors

Risk

This change adds one extra round trip to resolve activeUserIds before the flagged_accounts query. Trade-off is favourable: it removes the unbounded global load on every maintainer flagged-accounts page load, removes the timing side-channel surface, and aligns getFlaggedAccounts with every other maintainer action (which already scope at query level). The downstream profiles query and logMaintainerAction audit hook are unchanged.

@vercel

vercel Bot commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

@arcgod-design is attempting to deploy a commit to the codersogs-3057's projects Team on Vercel.

A member of the Team first needs to authorize it.

@pavsoss pavsoss left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please resolve the merge conflicts

@Ayush4958 Ayush4958 added the Needs author reply Author need to reply label Jul 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

HIGH: getFlaggedAccounts loads all global flagged accounts before client-side filtering — data leakage and scaling bottleneck

3 participants