sax3l/accounted
3
0
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases Wiki Activity

fix(security): tenant guard on the GL-line read RPCs (#625)

Browse Source 
* fix(security): tenant guard on the GL-line read RPCs (PR #624 follow-up)

get_unlinked_gl_lines and get_account_gl_lines_for_matching are SECURITY DEFINER
and EXECUTE-able by anon/authenticated, so any authenticated (or anonymous)
caller could invoke them directly over /rest/v1/rpc with another company's id and
read its general-ledger lines — a cross-tenant read that bypasses the API routes'
requireCompanyId() guard. Confirmed against the DB: anon and authenticated both
hold EXECUTE, and SECURITY DEFINER sidesteps RLS.

Add an in-function guard constraining anon/authenticated callers to their own
companies (the same boundary user_company_ids()/RLS enforces). Trusted callers
are untouched — service_role (the enable-banking reconciliation cron) and direct
/ superuser access (migrations, the pg-real harness) are not anon/authenticated,
so the predicate is a no-op and behaviour is unchanged. A foreign company id now
yields zero rows, not data.

Scope: hardens the two READ RPCs that expose ledger data. The remaining
company-scoped SECURITY DEFINER RPCs are writes / sequence generators with their
own internal authorization; a broader audit of that set is tracked separately.

pg-real coverage: a company-B member probing company A gets zero rows from both
RPCs, while a company-A member and direct/superuser access still see the data.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(security): revoke EXECUTE from PUBLIC/anon on the GL-line read RPCs

Defense-in-depth follow-up to the tenant guard. The guard already returns zero
rows to an anon/authenticated caller probing another company; this additionally
strips the EXECUTE privilege so an unauthenticated (anon) caller cannot invoke
the financial-ledger RPCs at all. Supabase grants EXECUTE to PUBLIC as well as to
anon, and anon is a member of PUBLIC — revoking only anon is insufficient, so
revoke both, then keep the two legitimate callers: authenticated (the API routes
call via the user's session; the in-function guard scopes them to their own
companies) and service_role (the enable-banking reconciliation cron).

Verified on the DB: anon EXECUTE = false, authenticated/service_role = true.
Adds an anon-role pg test asserting the call is rejected at the privilege layer.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(security): read JWT role from claims object in the GL-line RPC guard

The 20260611120000 guard used auth.role() to detect the caller's role, but
auth.role() reads the individual request.jwt.claim.role GUC first and only some
installs fall back to the claims object. PostgREST sets the claims OBJECT (the
individual claim.* GUCs are deprecated), and the pg-real harness sets
request.jwt.claims (+ claim.sub for auth.uid()) but NOT claim.role — so on an
auth.role() without the object fallback it returns NULL and the guard's
NOT IN ('anon','authenticated') branch was TRUE, skipping the membership check.
A pg-real test caught it: an authenticated non-member could still read another
company's GL lines (the guard failed open in that environment).

Read the role straight from request.jwt.claims (exactly what auth.role() itself
falls back to), so the guard enforces in every environment regardless of which
JWT-claim GUCs are populated. Verified on the DB: an authenticated non-member
evaluates both guard branches false → row excluded.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Jakob Wennberg
2026-06-02 10:24:18 +02:00
committed by GitHub
parent 094bd85e81
commit 358c25094d
4 changed files with 460 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files
supabase/migrations/20260611120000_gl_lines_rpc_tenant_guard.sql
+169
View File
@@ -0,0 +1,169 @@
-- Migration: tenant-isolation guard for the GL-line read RPCs.
--
-- get_unlinked_gl_lines and get_account_gl_lines_for_matching are SECURITY
-- DEFINER (they read journal_entry_lines / journal_entries / transactions as the
-- owner, bypassing RLS) and PostgREST grants EXECUTE to anon + authenticated.
-- Both take p_company_id as a plain argument, so any authenticated — or even
-- anonymous — caller could invoke them DIRECTLY over /rest/v1/rpc with another
-- company's id and read that company's general-ledger lines: a cross-tenant read
-- that bypasses the API routes' requireCompanyId() guard. Flagged in the PR #624
-- review and confirmed exploitable (anon and authenticated both hold EXECUTE,
-- and neither has rolbypassrls so RLS would normally protect the tables — but
-- SECURITY DEFINER sidesteps it).
--
-- Fix: enforce INSIDE the function that an anon/authenticated caller is a member
-- of p_company_id (the same boundary RLS enforces via user_company_ids()).
-- Trusted callers are NOT anon/authenticated and are deliberately left untouched:
-- * service_role — the enable-banking reconciliation cron calls
-- get_unlinked_gl_lines via the service client; its JWT role is
-- 'service_role' (and it has rolbypassrls).
-- * direct / superuser DB access — migrations and the pg-real test harness
-- call these on a bare connection with no JWT role claim.
-- For both, auth.role() is not 'anon'/'authenticated', so the added predicate is
-- a no-op and behaviour is byte-for-byte unchanged. Only the PostgREST-exposed
-- anon/authenticated path is constrained, to the caller's own companies. A
-- foreign p_company_id simply yields zero rows — no error, no data leak.
--
-- Scope: this hardens the two READ RPCs that expose ledger data. The remaining
-- company-scoped SECURITY DEFINER RPCs (commit_journal_entry, next_voucher_number,
-- delete_last_voucher, …) are writes / sequence generators with their own
-- internal authorization; a broader audit of that set is tracked separately.
-- ------------------------------------------------------------
-- get_unlinked_gl_lines — unchanged except the trailing tenant guard.
-- Body mirrors 20260605130000_unlinked_gl_lines_exclude_storno_correction.sql.
-- ------------------------------------------------------------
CREATE OR REPLACE FUNCTION public.get_unlinked_gl_lines(
p_company_id UUID,
p_account_number TEXT DEFAULT '1930',
p_date_from DATE DEFAULT NULL,
p_date_to DATE DEFAULT NULL
)
RETURNS TABLE (
line_id UUID,
journal_entry_id UUID,
debit_amount NUMERIC,
credit_amount NUMERIC,
line_description TEXT,
entry_date DATE,
voucher_number INT,
voucher_series TEXT,
entry_description TEXT,
source_type TEXT
)
LANGUAGE sql
STABLE
SECURITY DEFINER
SET search_path = public
AS $$
SELECT
jel.id AS line_id,
je.id AS journal_entry_id,
jel.debit_amount,
jel.credit_amount,
jel.line_description,
je.entry_date,
je.voucher_number,
je.voucher_series,
je.description AS entry_description,
je.source_type
FROM public.journal_entry_lines jel
JOIN public.journal_entries je ON je.id = jel.journal_entry_id
WHERE jel.account_number = p_account_number
AND je.company_id = p_company_id
AND je.status = 'posted'
AND je.source_type IS DISTINCT FROM 'opening_balance'
AND je.source_type IS DISTINCT FROM 'storno'
AND je.source_type IS DISTINCT FROM 'correction'
AND (p_date_from IS NULL OR je.entry_date >= p_date_from)
AND (p_date_to IS NULL OR je.entry_date <= p_date_to)
AND NOT EXISTS (
SELECT 1
FROM public.transactions t
WHERE t.journal_entry_id = je.id
AND t.company_id = p_company_id
)
-- Tenant guard (see migration header): anon/authenticated may only read
-- their own companies; service_role / direct DB access bypass.
AND (
coalesce(auth.role(), '') NOT IN ('anon', 'authenticated')
OR je.company_id IN (SELECT public.user_company_ids())
)
ORDER BY je.entry_date, je.voucher_number;
$$;
-- ------------------------------------------------------------
-- get_account_gl_lines_for_matching — unchanged except the trailing tenant guard.
-- Body mirrors 20260610120000_gl_lines_for_matching.sql.
-- ------------------------------------------------------------
CREATE OR REPLACE FUNCTION public.get_account_gl_lines_for_matching(
p_company_id UUID,
p_account_number TEXT DEFAULT '1930',
p_date_from DATE DEFAULT NULL,
p_date_to DATE DEFAULT NULL,
p_include_matched BOOLEAN DEFAULT false
)
RETURNS TABLE (
line_id UUID,
journal_entry_id UUID,
debit_amount NUMERIC,
credit_amount NUMERIC,
line_description TEXT,
entry_date DATE,
voucher_number INT,
voucher_series TEXT,
entry_description TEXT,
source_type TEXT,
linked_transaction_count INT
)
LANGUAGE sql
STABLE
SECURITY DEFINER
SET search_path = public
AS $$
SELECT
jel.id AS line_id,
je.id AS journal_entry_id,
jel.debit_amount,
jel.credit_amount,
jel.line_description,
je.entry_date,
je.voucher_number,
je.voucher_series,
je.description AS entry_description,
je.source_type,
(
SELECT count(*)
FROM public.transactions t
WHERE t.journal_entry_id = je.id
AND t.company_id = p_company_id
)::int AS linked_transaction_count
FROM public.journal_entry_lines jel
JOIN public.journal_entries je ON je.id = jel.journal_entry_id
WHERE jel.account_number = p_account_number
AND je.company_id = p_company_id
AND je.status = 'posted'
AND je.source_type IS DISTINCT FROM 'opening_balance'
AND je.source_type IS DISTINCT FROM 'storno'
AND je.source_type IS DISTINCT FROM 'correction'
AND (p_date_from IS NULL OR je.entry_date >= p_date_from)
AND (p_date_to IS NULL OR je.entry_date <= p_date_to)
AND (
p_include_matched
OR NOT EXISTS (
SELECT 1
FROM public.transactions t
WHERE t.journal_entry_id = je.id
AND t.company_id = p_company_id
)
)
-- Tenant guard (see migration header): anon/authenticated may only read
-- their own companies; service_role / direct DB access bypass.
AND (
coalesce(auth.role(), '') NOT IN ('anon', 'authenticated')
OR je.company_id IN (SELECT public.user_company_ids())
)
ORDER BY je.entry_date, je.voucher_number;
$$;
NOTIFY pgrst, 'reload schema';
supabase/migrations/20260611130000_gl_lines_rpc_revoke_public_execute.sql
+25
View File
@@ -0,0 +1,25 @@
-- Migration: least-privilege EXECUTE on the GL-line read RPCs.
--
-- Follow-up to 20260611120000_gl_lines_rpc_tenant_guard.sql. That migration's
-- in-function guard already closes the DATA leak — an anon/authenticated caller
-- gets zero rows for a company it doesn't belong to. This migration additionally
-- removes the unnecessary EXECUTE *privilege* for unauthenticated callers
-- (defense in depth / least privilege): anon should not even be able to invoke a
-- financial-ledger RPC.
--
-- Why REVOKE FROM anon alone is NOT enough: Supabase grants EXECUTE on new public
-- functions to PUBLIC *and* explicitly to anon/authenticated/service_role. anon
-- is a member of PUBLIC, so the PUBLIC grant keeps it callable even after its own
-- grant is revoked. Revoke both PUBLIC and anon, then (re)assert the only two
-- legitimate callers:
-- * authenticated — the API routes call via the user's session; the in-
-- function tenant guard scopes them to their own companies.
-- * service_role — the enable-banking reconciliation cron.
REVOKE EXECUTE ON FUNCTION public.get_unlinked_gl_lines(uuid, text, date, date) FROM PUBLIC, anon;
REVOKE EXECUTE ON FUNCTION public.get_account_gl_lines_for_matching(uuid, text, date, date, boolean) FROM PUBLIC, anon;
GRANT EXECUTE ON FUNCTION public.get_unlinked_gl_lines(uuid, text, date, date) TO authenticated, service_role;
GRANT EXECUTE ON FUNCTION public.get_account_gl_lines_for_matching(uuid, text, date, date, boolean) TO authenticated, service_role;
NOTIFY pgrst, 'reload schema';
supabase/migrations/20260611140000_gl_lines_rpc_guard_claims_read.sql
+153
View File
@@ -0,0 +1,153 @@
-- Migration: make the GL-line read-RPC tenant guard read the role from the JWT
-- claims object directly, instead of via auth.role().
--
-- 20260611120000 used `auth.role()` to detect the caller's role. auth.role()
-- reads the individual `request.jwt.claim.role` GUC first and only falls back to
-- the `request.jwt.claims` object on installs whose auth.role() carries that
-- fallback. PostgREST sets the claims OBJECT (the individual claim.* GUCs are
-- deprecated), and the pg-real test harness sets `request.jwt.claims` (+ an
-- individual `request.jwt.claim.sub` for auth.uid()) but NOT `request.jwt.claim.role`.
-- On an auth.role() without the object fallback that yields NULL, so the guard's
-- `NOT IN ('anon','authenticated')` branch was TRUE and the membership check was
-- skipped — i.e. the guard failed OPEN (caught by a pg-real test: an
-- authenticated non-member could still read another company's lines).
--
-- Reading `current_setting('request.jwt.claims', true)::jsonb ->> 'role'`
-- directly is what auth.role() itself falls back to, and depends only on the
-- claims object that both PostgREST and the harness reliably set — so the guard
-- enforces correctly in every environment. Behaviour is otherwise identical:
-- service_role and direct/superuser (no claims → NULL → '') still bypass; anon
-- and authenticated are constrained to their own companies.
CREATE OR REPLACE FUNCTION public.get_unlinked_gl_lines(
p_company_id UUID,
p_account_number TEXT DEFAULT '1930',
p_date_from DATE DEFAULT NULL,
p_date_to DATE DEFAULT NULL
)
RETURNS TABLE (
line_id UUID,
journal_entry_id UUID,
debit_amount NUMERIC,
credit_amount NUMERIC,
line_description TEXT,
entry_date DATE,
voucher_number INT,
voucher_series TEXT,
entry_description TEXT,
source_type TEXT
)
LANGUAGE sql
STABLE
SECURITY DEFINER
SET search_path = public
AS $$
SELECT
jel.id AS line_id,
je.id AS journal_entry_id,
jel.debit_amount,
jel.credit_amount,
jel.line_description,
je.entry_date,
je.voucher_number,
je.voucher_series,
je.description AS entry_description,
je.source_type
FROM public.journal_entry_lines jel
JOIN public.journal_entries je ON je.id = jel.journal_entry_id
WHERE jel.account_number = p_account_number
AND je.company_id = p_company_id
AND je.status = 'posted'
AND je.source_type IS DISTINCT FROM 'opening_balance'
AND je.source_type IS DISTINCT FROM 'storno'
AND je.source_type IS DISTINCT FROM 'correction'
AND (p_date_from IS NULL OR je.entry_date >= p_date_from)
AND (p_date_to IS NULL OR je.entry_date <= p_date_to)
AND NOT EXISTS (
SELECT 1
FROM public.transactions t
WHERE t.journal_entry_id = je.id
AND t.company_id = p_company_id
)
-- Tenant guard: anon/authenticated may only read their own companies;
-- service_role and direct/superuser access (no JWT role) bypass.
AND (
coalesce(nullif(current_setting('request.jwt.claims', true), '')::jsonb ->> 'role', '')
NOT IN ('anon', 'authenticated')
OR je.company_id IN (SELECT public.user_company_ids())
)
ORDER BY je.entry_date, je.voucher_number;
$$;
CREATE OR REPLACE FUNCTION public.get_account_gl_lines_for_matching(
p_company_id UUID,
p_account_number TEXT DEFAULT '1930',
p_date_from DATE DEFAULT NULL,
p_date_to DATE DEFAULT NULL,
p_include_matched BOOLEAN DEFAULT false
)
RETURNS TABLE (
line_id UUID,
journal_entry_id UUID,
debit_amount NUMERIC,
credit_amount NUMERIC,
line_description TEXT,
entry_date DATE,
voucher_number INT,
voucher_series TEXT,
entry_description TEXT,
source_type TEXT,
linked_transaction_count INT
)
LANGUAGE sql
STABLE
SECURITY DEFINER
SET search_path = public
AS $$
SELECT
jel.id AS line_id,
je.id AS journal_entry_id,
jel.debit_amount,
jel.credit_amount,
jel.line_description,
je.entry_date,
je.voucher_number,
je.voucher_series,
je.description AS entry_description,
je.source_type,
(
SELECT count(*)
FROM public.transactions t
WHERE t.journal_entry_id = je.id
AND t.company_id = p_company_id
)::int AS linked_transaction_count
FROM public.journal_entry_lines jel
JOIN public.journal_entries je ON je.id = jel.journal_entry_id
WHERE jel.account_number = p_account_number
AND je.company_id = p_company_id
AND je.status = 'posted'
AND je.source_type IS DISTINCT FROM 'opening_balance'
AND je.source_type IS DISTINCT FROM 'storno'
AND je.source_type IS DISTINCT FROM 'correction'
AND (p_date_from IS NULL OR je.entry_date >= p_date_from)
AND (p_date_to IS NULL OR je.entry_date <= p_date_to)
AND (
p_include_matched
OR NOT EXISTS (
SELECT 1
FROM public.transactions t
WHERE t.journal_entry_id = je.id
AND t.company_id = p_company_id
)
)
-- Tenant guard: anon/authenticated may only read their own companies;
-- service_role and direct/superuser access (no JWT role) bypass.
AND (
coalesce(nullif(current_setting('request.jwt.claims', true), '')::jsonb ->> 'role', '')
NOT IN ('anon', 'authenticated')
OR je.company_id IN (SELECT public.user_company_ids())
)
ORDER BY je.entry_date, je.voucher_number;
$$;
NOTIFY pgrst, 'reload schema';
tests/pg/gl_lines_rpc_tenant_guard.pg.test.ts
+113
View File
@@ -0,0 +1,113 @@
/**
* pg-real test for the GL-line read-RPC tenant guard
* (20260611120000_gl_lines_rpc_tenant_guard.sql).
*
* get_unlinked_gl_lines and get_account_gl_lines_for_matching are SECURITY
* DEFINER and EXECUTE-able by anon/authenticated, so without the guard any
* authenticated user could call them directly with another company's id and read
* its general ledger. The guard enforces membership for anon/authenticated while
* leaving service_role and direct/superuser access (this harness, migrations,
* the reconciliation cron) untouched.
*/
import { describe, it, expect } from 'vitest'
import { randomUUID } from 'node:crypto'
import { getPool, withUserContext } from './setup'
import { seedCompany } from './fixtures'
async function insertPostedJournalEntry(params: {
userId: string
companyId: string
fiscalPeriodId: string
entryDate: string
voucherNumber: number
amount?: number
}): Promise<string> {
const id = randomUUID()
const amount = params.amount ?? 1500
await getPool().query(
`INSERT INTO public.journal_entries
(id, user_id, company_id, fiscal_period_id, voucher_number, voucher_series,
entry_date, description, source_type, status)
VALUES ($1, $2, $3, $4, $5, 'A', $6, 'Bank tx', 'bank_transaction', 'posted')`,
[id, params.userId, params.companyId, params.fiscalPeriodId, params.voucherNumber, params.entryDate],
)
await getPool().query(
`INSERT INTO public.journal_entry_lines
(journal_entry_id, account_number, debit_amount, credit_amount)
VALUES ($1, '1930', $2, 0),
($1, '2091', 0, $2)`,
[id, amount],
)
return id
}
const UNLINKED = `SELECT journal_entry_id FROM public.get_unlinked_gl_lines($1)`
const MATCHING = `SELECT journal_entry_id FROM public.get_account_gl_lines_for_matching($1, '1930', NULL, NULL, true)`
describe('GL-line read RPCs — tenant-isolation guard', () => {
it('lets a member read its own company but blocks an authenticated non-member', async () => {
const a = await seedCompany() // userA is owner-member of companyA
const b = await seedCompany() // userB belongs to companyB only
const entryA = await insertPostedJournalEntry({
userId: a.userId,
companyId: a.companyId,
fiscalPeriodId: a.fiscalPeriodId,
entryDate: '2026-03-15',
voucherNumber: 1,
})
// Direct / superuser connection (no JWT role) — the trusted bypass that this
// harness, migrations and the service-role cron rely on. Data is visible.
const bare = await getPool().query(UNLINKED, [a.companyId])
expect(bare.rows.map((r) => r.journal_entry_id)).toContain(entryA)
// A member of company A sees A's line through both RPCs.
await withUserContext(a.userId, async (client) => {
const u = await client.query(UNLINKED, [a.companyId])
expect(u.rows.map((r) => r.journal_entry_id)).toContain(entryA)
const m = await client.query(MATCHING, [a.companyId])
expect(m.rows.map((r) => r.journal_entry_id)).toContain(entryA)
})
// A member of company B probing company A gets nothing — the cross-tenant
// read that SECURITY DEFINER + anon/authenticated EXECUTE used to allow.
await withUserContext(b.userId, async (client) => {
const u = await client.query(UNLINKED, [a.companyId])
expect(u.rows).toHaveLength(0)
const m = await client.query(MATCHING, [a.companyId])
expect(m.rows).toHaveLength(0)
})
})
it('rejects an anon (unauthenticated) caller outright — EXECUTE revoked from anon + PUBLIC', async () => {
const a = await seedCompany()
await insertPostedJournalEntry({
userId: a.userId,
companyId: a.companyId,
fiscalPeriodId: a.fiscalPeriodId,
entryDate: '2026-03-15',
voucherNumber: 1,
})
// Each probe runs in its own transaction: a permission-denied error aborts
// the transaction, so the two can't share one. anon has no EXECUTE (revoked
// from its own grant AND from PUBLIC, of which anon is a member), so the call
// is rejected at the privilege layer — defense in depth on top of the
// in-function tenant guard.
const callAsAnon = async (sql: string): Promise<void> => {
const client = await getPool().connect()
try {
await client.query('BEGIN')
await client.query(`SELECT set_config('request.jwt.claims', '{"role":"anon"}', true)`)
await client.query('SET LOCAL ROLE anon')
await client.query(sql, [a.companyId])
} finally {
await client.query('ROLLBACK').catch(() => {})
client.release()
}
}
await expect(callAsAnon(UNLINKED)).rejects.toThrow(/permission denied/i)
await expect(callAsAnon(MATCHING)).rejects.toThrow(/permission denied/i)
})
})