fix(storage): drop the client-side DELETE policy on the documents bucket (#1254)
* fix(storage): drop the client-side DELETE policy on the documents bucket
20240101000024 documents this bucket as WORM: "No UPDATE or DELETE policies".
That described the repo, not production. Production carries a
users_delete_own_documents policy that exists in no migration file:
FOR DELETE TO authenticated
USING (bucket_id = 'documents'
AND (storage.foldername(name))[2] = auth.uid()::text)
Under it, the uploading user can delete the storage bytes of any document
they uploaded under the legacy documents/{userId}/... layout, using nothing
but their normal browser token. That includes documents linked to a posted
verifikat, which are rakenskapsinformation under the BFL 7 kap 2 § seven-year
retention duty. deleteDocument()'s linked-check and the
block_document_deletion() trigger both guard the document_attachments ROW,
not the object: the row survives, still pointing at a file that is gone.
Reproduced against a local replay of the full migration stream: with the
policy present the uploader's own DELETE removes the object; with it dropped
the same statement matches zero rows. Company-scoped keys were never exposed
(their second path segment is the company id, not auth.uid()), so this only
ever reached the legacy layout, which is where most documents still live.
Safe because every in-app remove() on this bucket already runs on the service
role, covered by service_role_all_documents.
Deliberately narrow: users_read_own_documents and users_upload_own_documents
stay. The Phase B backfill from 20260726092000 has not run, so dropping the
legacy SELECT policy now would make existing documents unreadable. That is
Phase C.
The pg-real test asserts no DELETE and no UPDATE policy over the bucket under
ANY name: the hole arrived under a name this repo never used, so pinning a
name would not have caught it.
Refs #1208
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* test(storage): make the WORM ratchet see FOR ALL and WITH CHECK policies
Review caught two blind spots in the ratchet, both fair. It matched only
polcmd 'd' and 'w', but polcmd '*' (FOR ALL) grants DELETE and UPDATE just as
effectively, and FOR ALL is the shape the one legitimate policy on this table
already uses, so a hostile one would look unremarkable in the catalogue. It
also read only polqual, so an UPDATE policy carrying its bucket restriction in
WITH CHECK was invisible.
Both assertions now run through one helper that covers d/w/*, concatenates
USING and WITH CHECK, and filters by grantee so service_role_all_documents
(how the application does its authorized deletes) is excluded while every
client-reachable role is not. A policy granted to PUBLIC has an empty
polroles, which is the most permissive case there is, so it is treated as
client-reachable rather than as "no roles".
Matching on the substring rather than the exact `bucket_id = 'documents'`
shape pg_get_expr emits today: a policy written as bucket_id::text or with the
comparison reversed would slip past a stricter match, and for a WORM ratchet a
false alarm is cheap while a silent hole is not.
Adds a probe case that creates a FOR ALL policy and asserts the helper sees
it, so the main assertion cannot pass vacuously. That case earned its keep
immediately: it caught that node-postgres hands back a raw string for a name[]
column, so the role filter needed rolname::text to work at all.
Verified against a local replay of the full migration stream: red with the
original prod FOR DELETE policy present, red with a FOR ALL probe, green
without either. Full pg-real suite 933 passed.
Refs #1208
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* test(storage): catch a destructive policy that names no bucket at all
Adversarial review of the previous commit found the ratchet still failed
open, and reproduced it: a policy with no bucket_id predicate covers EVERY
bucket, documents included, so gating on the bucket name discarded exactly
the widest hole. The concrete shape is Supabase's own stock "Enable delete for
users based on user_id" template, USING (auth.uid() = owner), which is the
single most likely form of a future dashboard edit.
A destructive policy is now in scope unless it provably cannot reach this
bucket, i.e. only a bucket_id predicate naming some other bucket exempts it.
The behavioural assertions had the matching blind spot: fixtures were seeded
without an owner, so an owner-based policy matched NULL and the DELETE
reported 0 rows for the wrong reason. Objects now carry an owner the way
storage-api stamps them in production, so those tests fail loudly instead of
passing by accident.
Two probes pin both directions: a bucketless policy must be reported (and is
shown to really permit the delete), and a policy scoped to another bucket must
not be, so the ratchet cannot start crying wolf on receipts or sie-files and
get switched off.
Verified against a local replay of the full migration stream: red with the
stock bucketless template installed, green without it. Full pg-real suite 935
passed.
Refs #1208
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Jakob Wennberg
co-authored by
Claude Opus 5
Claude Opus 5
parent
579f48752e
commit
ece6eca922
@@ -630,3 +630,4 @@ One line per decision: `[YYYY-MM-DD] <decision>: <why>`. Appended by agents and
|
||||
[2026-07-27] Docs export (#1247): scripts/export-docs-to-website.mts stubs `server-only` via a Module._load hook instead of untangling the import chain. The chain is real (lib/api/v1/load-routes -> every v1 route -> lib/init -> posthog-observability -> posthog-server) and the script only reads exported markdown builders, so breaking the chain would mean restructuring route imports for a build-time script's benefit.
|
||||
[2026-07-27] Webhook delivery latency (#1201): implemented option (a), an emit-triggered kick of the existing dispatchDueDeliveries, not option (b), an authenticated SSE stream over event_log. The kick is a new lib/webhooks/dispatch-kick.ts wired into fanOutToWebhooks plus the two routes that enqueue a delivery directly (the :test verb and the manual delivery retry), and it does NOT close #1201: the issue asks for a realtime stream for API consumers and that remains open. Three constraints shaped it. It is never awaited: eventBus.emit is awaited at ~99 call sites including journal_entry.committed, and each delivery can burn a 10 s receiver timeout, so awaiting would put a stranger's HTTP endpoint on the critical path of committing a verifikat. It is coalesced per function instance, because a bulk operation emits once per row and would otherwise schedule one claim round trip per row. Its batch size is 5 rather than the cron's 50, because this work runs on the tail of a user-facing request. The SKIP LOCKED claim keeps a kick and the cron from claiming the same row at the same moment, but that is a claim-time guarantee only: the RPC autocommits before any POST, so a later cycle's recoverStuckInFlight sweep can re-arm a row still queued behind an earlier serial loop. Delivery stays at-least-once as the public docs already promise, and the kick's batch of 5 opens a narrower window than the cron's existing batch of 50 against the same 20 s stuck threshold; an early draft of this change claimed double delivery was impossible, which was wrong and is now corrected in the code comments. Scheduling uses next/server after() with a deferred-microtask fallback outside a request scope, mirroring the enable-banking callback; the fallback must stay deferred rather than inline, or the coalescing flag clears before the next kick in the same tick can see it. No API_V1_VERSION bump: no new event types and no payload change, only latency.
|
||||
[2026-07-27] Bolagsskatt add-back (#1051): sumPostedYearEndDispositions now adds back 78xx planenlig avskrivning alongside 88xx and 7533, and excludes fiscal_periods.closing_entry_id from its fetch. Shipping only this "Stage 1" half of the issue: it corrects the tax base and the periodiseringsfond 25 % cap with no migration and no displayed-figure change. The issue's other half (making /rapporter show bokslut entries by moving generateIncomeStatement to excludeFinalClosingEntry) is deliberately NOT done here: it duplicates the exclusion in the kpi_report_aggregates RPC (so it needs a migration plus a pg test), it changes displayed profit for every company that ran the bokslut flow, and it requires removing the add-back at four call sites, including the one that caused the original too-high-tax customer bug. The closing-entry exclusion is part of Stage 1 rather than a follow-up because closing verifikat do carry 78xx/88xx/7533 reversal lines on production, so without it the new add-back silently cancels itself once the year is closed. The issue's stated constraint that source_type='year_end' is load-bearing for the iXBRL RR/BR split is stale: build-input.ts and arsredovisning/build-data.ts already moved to excludeFinalClosingEntry.
|
||||
[2026-07-27] Documents bucket WORM (#1208): dropped the production-only `users_delete_own_documents` DELETE policy on storage.objects and pinned the invariant with a name-agnostic pg-real test, rather than adding a storage DELETE policy for authenticated users or building the orphan-cleanup script the issue asks for. The policy existed in no migration (dashboard drift, alongside `users_read_own_documents` / `users_upload_own_documents`, which production has INSTEAD of this repo's `documents_select_own` / `documents_insert_own`) and let any user delete, with a normal browser token, the storage bytes of documents linked to posted verifikat: rakenskapsinformation under BFL 7 kap 2 §. Neither deleteDocument()'s linked-check nor block_document_deletion() reaches that far; both protect the row, and the row survives pointing at nothing. Verified reproducible against a local replay of the full migration stream: with the policy present the uploader's own DELETE removes a legacy-layout object, with it dropped the DELETE matches zero rows. Dropping it breaks nothing because every in-app remove() on this bucket has run service-role since #1215. The two legacy read/insert policies are deliberately left alone: the Phase B backfill from 20260726092000 has not run, so dropping the legacy SELECT would make most existing documents unreadable. That is Phase C. The 326 orphan objects (49.5 MB) the issue also describes are NOT cleaned up here: irreversible deletion against 7-year-retention data is not worth 49 MB without a separate report-only pass. The test asserts no DELETE and no UPDATE policy over the bucket under ANY name, because the hole arrived under a name this repo never used.
|
||||
|
||||
@@ -0,0 +1,65 @@
|
||||
-- Close a client-side DELETE hole in the `documents` storage bucket.
|
||||
--
|
||||
-- THE HOLE
|
||||
--
|
||||
-- 20240101000024_storage_bucket_policies.sql documents the bucket as WORM:
|
||||
-- "No UPDATE or DELETE policies - WORM compliance". That comment describes
|
||||
-- the repo, not production. Production carries a DELETE policy on
|
||||
-- storage.objects that exists in NO migration file:
|
||||
--
|
||||
-- users_delete_own_documents
|
||||
-- FOR DELETE TO authenticated
|
||||
-- USING (bucket_id = 'documents'
|
||||
-- AND (storage.foldername(name))[2] = auth.uid()::text)
|
||||
--
|
||||
-- It was created outside the migration stream (dashboard drift), alongside
|
||||
-- `users_read_own_documents` and `users_upload_own_documents`, which likewise
|
||||
-- appear in no migration and which production has INSTEAD of this repo's
|
||||
-- `documents_select_own` / `documents_insert_own`.
|
||||
--
|
||||
-- Under that policy the uploading user, holding nothing but their normal
|
||||
-- browser token, can DELETE the storage bytes of every document they uploaded
|
||||
-- under the legacy `documents/{userId}/...` layout: including documents
|
||||
-- linked to a posted verifikat, i.e. rakenskapsinformation under the BFL
|
||||
-- 7 kap 2 § seven-year retention duty. None of the three guards we rely on
|
||||
-- reach that far: deleteDocument()'s linked-check and the
|
||||
-- block_document_deletion() trigger both protect the document_attachments
|
||||
-- ROW, and the row survives, still pointing at an object that is gone.
|
||||
--
|
||||
-- WHY DROPPING IT IS SAFE
|
||||
--
|
||||
-- Nothing in the application deletes a documents object with a user-bound
|
||||
-- client. Every remove() call on this bucket runs on the service role, which
|
||||
-- is covered by service_role_all_documents and unaffected by this drop:
|
||||
-- lib/core/documents/document-service.ts uploadDocument cleanup,
|
||||
-- createNewVersion cleanup,
|
||||
-- deleteDocument
|
||||
-- -> all three on createServiceClientNoCookies() since #1215
|
||||
-- extensions/general/mcp-server/server.ts audit-package sign-failure cleanup
|
||||
-- -> MCP paths already hold a service-role client
|
||||
-- scripts/backfill-document-storage-paths.ts
|
||||
-- -> builds its own service-role client
|
||||
--
|
||||
-- SCOPE
|
||||
--
|
||||
-- Only the DELETE policy goes. `users_read_own_documents` and
|
||||
-- `users_upload_own_documents` stay: the Phase B backfill in
|
||||
-- 20260726092000_documents_bucket_company_scope.sql has not run, the large
|
||||
-- majority of document_attachments rows are still on the legacy key layout,
|
||||
-- and dropping the legacy SELECT policy now would make those documents
|
||||
-- unreadable. Retiring them is Phase C, gated on the backfill reporting zero
|
||||
-- legacy-prefix objects, and it is deliberately not bundled here.
|
||||
--
|
||||
-- IF EXISTS, because the policy is production-only: on a fresh install, a
|
||||
-- preview branch or a self-hosted database this is a no-op, which is the
|
||||
-- correct outcome. tests/pg/documents-bucket-worm.pg.test.ts pins the
|
||||
-- invariant so a DELETE policy over this bucket cannot come back through a
|
||||
-- migration unnoticed.
|
||||
--
|
||||
-- Refs: #1208, #1207, #1215, 20260726092000_documents_bucket_company_scope.sql
|
||||
|
||||
DROP POLICY IF EXISTS users_delete_own_documents ON storage.objects;
|
||||
|
||||
-- Defensive: the same hole under this repo's own naming convention. Never
|
||||
-- created by any migration here, but a dashboard edit could have used it.
|
||||
DROP POLICY IF EXISTS documents_delete_own ON storage.objects;
|
||||
@@ -0,0 +1,254 @@
|
||||
import { afterAll, beforeAll, describe, expect, it } from 'vitest'
|
||||
import { getPool, withUserContext } from './setup'
|
||||
import { insertAuthUser, insertCompany, insertCompanyMember } from './fixtures'
|
||||
|
||||
/**
|
||||
* WORM ratchet for the `documents` storage bucket, after
|
||||
* 20260727190000_drop_documents_bucket_delete_policy.sql.
|
||||
*
|
||||
* The bug this guards: production carried a `users_delete_own_documents`
|
||||
* policy (FOR DELETE TO authenticated, USING bucket_id = 'documents' AND
|
||||
* (storage.foldername(name))[2] = auth.uid()::text) that existed in no
|
||||
* migration file. It let the uploading user delete the storage bytes of any
|
||||
* document they had uploaded under the legacy `documents/{userId}/...`
|
||||
* layout, including documents linked to a posted verifikat: those are
|
||||
* rakenskapsinformation under the BFL 7 kap 2 § seven-year retention duty.
|
||||
*
|
||||
* The application-layer guard in deleteDocument() and the
|
||||
* block_document_deletion() trigger both protect the document_attachments
|
||||
* ROW, not the object, so neither one closes this. Deletion of a documents
|
||||
* object must stay a server-side, service-role code path.
|
||||
*
|
||||
* These assertions are name-agnostic on purpose: the hole arrived under a
|
||||
* name this repo never used, so pinning a name would not have caught it.
|
||||
*/
|
||||
describe('documents bucket: WORM (no client-side DELETE)', () => {
|
||||
const objectNames: string[] = []
|
||||
|
||||
let owner: string
|
||||
let company: string
|
||||
let legacyKey: string
|
||||
let companyScopedKey: string
|
||||
|
||||
async function seedObject(name: string): Promise<void> {
|
||||
// owner is populated deliberately: storage-api stamps the uploader there
|
||||
// in production, and an owner-based policy (`USING (auth.uid() = owner)`,
|
||||
// Supabase's stock delete template) matches nothing when it is NULL. A
|
||||
// fixture without an owner would let that policy shape pass this suite by
|
||||
// comparing against NULL rather than by being absent.
|
||||
await getPool().query(
|
||||
`INSERT INTO storage.objects (bucket_id, name, owner) VALUES ('documents', $1, $2)`,
|
||||
[name, owner],
|
||||
)
|
||||
objectNames.push(name)
|
||||
}
|
||||
|
||||
beforeAll(async () => {
|
||||
await getPool().query(
|
||||
`INSERT INTO storage.buckets (id, name, public)
|
||||
VALUES ('documents', 'documents', false)
|
||||
ON CONFLICT (id) DO NOTHING`,
|
||||
)
|
||||
|
||||
// Real Supabase grants these to `authenticated`; the bare CI image may
|
||||
// not. Without the DELETE grant the deletion assertion below would pass
|
||||
// for the wrong reason (permission denied, not RLS).
|
||||
await getPool()
|
||||
.query(`GRANT SELECT, INSERT, DELETE ON storage.objects TO authenticated`)
|
||||
.catch(() => {})
|
||||
|
||||
owner = await insertAuthUser()
|
||||
company = await insertCompany({ createdBy: owner, name: 'WORM Test AB' })
|
||||
await insertCompanyMember({ companyId: company, userId: owner, role: 'owner' })
|
||||
|
||||
legacyKey = `documents/${owner}/1700000000100_kvitto.pdf`
|
||||
companyScopedKey = `documents/${company}/${owner}/1700000000101_kvitto.pdf`
|
||||
await seedObject(legacyKey)
|
||||
await seedObject(companyScopedKey)
|
||||
})
|
||||
|
||||
afterAll(async () => {
|
||||
const sweep = (sql: string, params: unknown[]) =>
|
||||
getPool()
|
||||
.query(sql, params)
|
||||
.catch(() => {})
|
||||
|
||||
if (objectNames.length > 0) {
|
||||
await sweep(`DELETE FROM storage.objects WHERE name = ANY($1::text[])`, [objectNames])
|
||||
}
|
||||
await sweep(`DELETE FROM public.company_members WHERE company_id = $1`, [company])
|
||||
await sweep(`DELETE FROM public.companies WHERE id = $1`, [company])
|
||||
await sweep(`DELETE FROM auth.users WHERE id = $1`, [owner])
|
||||
})
|
||||
|
||||
/**
|
||||
* Every policy on storage.objects that can destroy or rewrite an object and
|
||||
* is not demonstrably scoped away from the documents bucket, with both its
|
||||
* USING and WITH CHECK expressions and its grantee roles.
|
||||
*
|
||||
* polcmd '*' (FOR ALL) is included deliberately: it grants DELETE and
|
||||
* UPDATE just as effectively as 'd' and 'w', and it is the shape the one
|
||||
* legitimate policy here (service_role_all_documents) already uses, so a
|
||||
* hostile FOR ALL policy would look unremarkable in the catalogue.
|
||||
*
|
||||
* WITH CHECK is read as well as USING: an UPDATE policy can carry its
|
||||
* bucket restriction in either, and a policy whose USING is permissive
|
||||
* would be invisible to a polqual-only check.
|
||||
*/
|
||||
async function destructivePoliciesOverDocuments(): Promise<string[]> {
|
||||
const res = await getPool().query<{
|
||||
polname: string
|
||||
cmd: string
|
||||
expr: string
|
||||
roles: string[]
|
||||
}>(
|
||||
`SELECT p.polname,
|
||||
p.polcmd::text AS cmd,
|
||||
coalesce(pg_get_expr(p.polqual, p.polrelid), '')
|
||||
|| ' ' || coalesce(pg_get_expr(p.polwithcheck, p.polrelid), '') AS expr,
|
||||
-- ::text is load-bearing: rolname is of type name, and
|
||||
-- node-postgres hands back a raw "{authenticated}" string for a
|
||||
-- name[] column instead of parsing it into a JS array.
|
||||
ARRAY(SELECT rolname::text FROM pg_roles WHERE oid = ANY (p.polroles)) AS roles
|
||||
FROM pg_policy p
|
||||
WHERE p.polrelid = 'storage.objects'::regclass
|
||||
AND p.polcmd IN ('d', 'w', '*')`,
|
||||
)
|
||||
|
||||
return res.rows
|
||||
.filter((r) => {
|
||||
// In scope unless the policy provably cannot reach this bucket. Only
|
||||
// a bucket_id predicate naming some OTHER bucket exempts it: an
|
||||
// expression that never mentions bucket_id covers every bucket,
|
||||
// documents included. That is exactly Supabase's stock "Enable delete
|
||||
// for users based on user_id" template, `USING (auth.uid() = owner)`,
|
||||
// which carries no bucket clause at all, so gating on the bucket name
|
||||
// alone would wave through the widest possible hole.
|
||||
//
|
||||
// The bucket check is a substring rather than the exact
|
||||
// `bucket_id = 'documents'` shape pg_get_expr emits today, since
|
||||
// `bucket_id::text = 'documents'` or a reversed comparison would slip
|
||||
// past a stricter match. For a WORM ratchet a false alarm is cheap and
|
||||
// a silent hole is not.
|
||||
if (!r.expr.includes('documents') && r.expr.includes('bucket_id')) return false
|
||||
// An empty role array means the policy is granted to PUBLIC (oid 0
|
||||
// has no pg_roles row), which is the most permissive case there is,
|
||||
// so it must NOT be read as "no client roles".
|
||||
if (r.roles.length === 0) return true
|
||||
// service_role bypasses RLS anyway and is how the application does
|
||||
// its authorized deletes; every other grantee is client-reachable.
|
||||
return r.roles.some((role) => role !== 'service_role')
|
||||
})
|
||||
.map((r) => `${r.polname} (${r.cmd})`)
|
||||
}
|
||||
|
||||
it('no client-reachable DELETE, UPDATE or FOR ALL policy covers the documents bucket', async () => {
|
||||
// receipts_delete is a different bucket and is intentionally deletable:
|
||||
// receipts are pre-bookkeeping scratch, not rakenskapsinformation. An
|
||||
// UPDATE policy would be as damaging as a DELETE one: it lets a user
|
||||
// rewrite an object in place, defeating the version chain.
|
||||
expect(await destructivePoliciesOverDocuments()).toEqual([])
|
||||
})
|
||||
|
||||
it('the ratchet sees a FOR ALL policy, which is how the real hole could return', async () => {
|
||||
// Proves the assertion above is not vacuous. The dropped policy was
|
||||
// FOR DELETE, but nothing stops the next dashboard edit from being
|
||||
// FOR ALL, and that is the shape a polcmd IN ('d','w') check misses.
|
||||
await getPool().query(
|
||||
`CREATE POLICY worm_ratchet_probe ON storage.objects
|
||||
FOR ALL TO authenticated
|
||||
USING (bucket_id = 'documents')
|
||||
WITH CHECK (bucket_id = 'documents')`,
|
||||
)
|
||||
try {
|
||||
expect(await destructivePoliciesOverDocuments()).toEqual(['worm_ratchet_probe (*)'])
|
||||
} finally {
|
||||
await getPool().query(`DROP POLICY IF EXISTS worm_ratchet_probe ON storage.objects`)
|
||||
}
|
||||
// ... and the catalogue is clean again once the probe is gone.
|
||||
expect(await destructivePoliciesOverDocuments()).toEqual([])
|
||||
})
|
||||
|
||||
it('the ratchet sees a bucketless policy, which covers documents by omission', async () => {
|
||||
// Supabase's stock "Enable delete for users based on user_id" template is
|
||||
// `USING (auth.uid() = owner)` with no bucket clause, so it grants delete
|
||||
// over EVERY bucket. Naming no bucket must not read as naming a safe one.
|
||||
await getPool().query(
|
||||
`CREATE POLICY worm_ratchet_bucketless ON storage.objects
|
||||
FOR DELETE TO authenticated
|
||||
USING (auth.uid() = owner)`,
|
||||
)
|
||||
try {
|
||||
expect(await destructivePoliciesOverDocuments()).toEqual([
|
||||
'worm_ratchet_bucketless (d)',
|
||||
])
|
||||
|
||||
// And it is not merely reported: it really would let the uploader
|
||||
// destroy their own rakenskapsinformation, which is why the catalogue
|
||||
// assertion has to catch it.
|
||||
await withUserContext(owner, async (client) => {
|
||||
const res = await client.query(
|
||||
`DELETE FROM storage.objects WHERE bucket_id = 'documents' AND name = $1`,
|
||||
[legacyKey],
|
||||
)
|
||||
expect(res.rowCount).toBe(1)
|
||||
})
|
||||
} finally {
|
||||
await getPool().query(`DROP POLICY IF EXISTS worm_ratchet_bucketless ON storage.objects`)
|
||||
}
|
||||
expect(await destructivePoliciesOverDocuments()).toEqual([])
|
||||
})
|
||||
|
||||
it('a policy scoped to another bucket is not flagged', async () => {
|
||||
// The counterweight to the rule above: receipts_delete is real, lives in
|
||||
// migration 20260710102000, and must not trip this ratchet. A ratchet
|
||||
// that cries wolf on unrelated buckets gets switched off.
|
||||
await getPool().query(
|
||||
`CREATE POLICY worm_ratchet_other_bucket ON storage.objects
|
||||
FOR DELETE TO authenticated
|
||||
USING (bucket_id = 'sie-files')`,
|
||||
)
|
||||
try {
|
||||
expect(await destructivePoliciesOverDocuments()).toEqual([])
|
||||
} finally {
|
||||
await getPool().query(`DROP POLICY IF EXISTS worm_ratchet_other_bucket ON storage.objects`)
|
||||
}
|
||||
})
|
||||
|
||||
it('the uploader cannot delete their own legacy-layout object', async () => {
|
||||
// The exact production shape: [2] of `documents/{userId}/...` is the
|
||||
// uploader's auth.uid(), which is what the dropped policy matched on.
|
||||
await withUserContext(owner, async (client) => {
|
||||
const res = await client.query(
|
||||
`DELETE FROM storage.objects WHERE bucket_id = 'documents' AND name = $1`,
|
||||
[legacyKey],
|
||||
)
|
||||
// RLS filters the row out rather than raising: the DELETE reports
|
||||
// success having removed nothing. That silence is why the hole was
|
||||
// invisible from the application side.
|
||||
expect(res.rowCount).toBe(0)
|
||||
})
|
||||
|
||||
const after = await getPool().query(
|
||||
`SELECT 1 FROM storage.objects WHERE bucket_id = 'documents' AND name = $1`,
|
||||
[legacyKey],
|
||||
)
|
||||
expect(after.rowCount).toBe(1)
|
||||
})
|
||||
|
||||
it('a company member cannot delete a company-scoped object either', async () => {
|
||||
await withUserContext(owner, async (client) => {
|
||||
const res = await client.query(
|
||||
`DELETE FROM storage.objects WHERE bucket_id = 'documents' AND name = $1`,
|
||||
[companyScopedKey],
|
||||
)
|
||||
expect(res.rowCount).toBe(0)
|
||||
})
|
||||
|
||||
const after = await getPool().query(
|
||||
`SELECT 1 FROM storage.objects WHERE bucket_id = 'documents' AND name = $1`,
|
||||
[companyScopedKey],
|
||||
)
|
||||
expect(after.rowCount).toBe(1)
|
||||
})
|
||||
})
|
||||
Reference in New Issue
Block a user