Blog · July 31, 2026

The write RPC for a file attachment never checked whose file it was

Every file attachment on Ledgenter lives in a shared storage bucket, partitioned by convention: a storage_path is supposed to start with <tenant_id>/<object_type>/<object_id>/<filename>. The bucket's row-level security (migration 0008) already enforces this — a request for bytes at a path that doesn't start with the caller's own tenant_id is denied, full stop, at the storage layer.

attach_add, the RPC that registers an attachment against a project, task, decision, note, comment, or handoff, never enforced the same convention on the way in.

if p_kind = 'file' and coalesce(nullif(p_storage_path, ''), null) is null then
  raise exception 'kind=file requires storage_path' using errcode = '23514';
end if;
-- ...no check here that p_storage_path is scoped to the caller's own tenant
insert into public.attachments (tenant_id, object_type, object_id, kind, ..., storage_path, ...)
values (v_tenant, p_object_type, p_object_id, p_kind, ..., p_storage_path, ...);

It required a non-empty string. It did not require that string to be yours. A caller could register an attachment row pointing at any storage_path at all — a guessed path, a leaked one, another tenant's — and attach_add would write it without complaint.

Why "the bucket already blocks it" wasn't the whole answer

Read narrowly, this looks harmless: the bucket RLS is the thing that actually gates a byte read, and it doesn't care what attachments rows exist — it only cares whether the fetch is scoped correctly. Register a row pointing at someone else's path, and today, nothing in this repo ever turns that row back into a signed URL or a download. The gap is real but inert.

That's exactly the shape of bug that stops being inert the moment someone adds the next feature. A signed-URL helper, a download endpoint, an attachment preview — any of them would reasonably read storage_path off the attachments row and hand it to the storage client, trusting that a row which exists at all must have passed some check on the way in. It wouldn't be wrong to assume that. It just wouldn't be true yet. The first helper that makes that assumption turns a silently-accepted bad row into a live cross-tenant disclosure path, and the bug it inherits won't look like a bug in the new code — it'll look like a gap in attach_add, discovered the hard way instead of the easy one.

The fix mirrors the convention it was supposed to enforce

if p_kind = 'file' and (split_part(p_storage_path, '/', 1) <> v_tenant::text) then
  raise exception 'storage_path must be scoped to the caller''s own tenant' using errcode = '23514';
end if;

One check, placed where the RPC already validates everything else about the payload before it ever reaches the insert. It reads the first path segment and compares it against the caller's own tenant_id from the signed claim — the same value the bucket RLS policy checks, checked here too, so the write path and the read path finally agree on what's allowed before either of them is trusted alone.

Deliberately left out: verifying the path actually exists in storage.objects. That would mean attach_add reading across the storage RLS boundary, and it risks rejecting a legitimate upload-then-register sequence if the object hasn't finished landing yet — a real question, but a different one, that needs a live upload flow to test against rather than a body-only migration. This fix closes the part that was actually wrong: a tenant boundary that wasn't checked, not a race that wasn't handled.

Same signature, same return shape as before — CREATE OR REPLACE, no contract drift, safe to ship straight to master.

Found by finally reading the whole RPC, not just its happy path

attach_add already had an existence check — it correctly verified the parent object (the project, task, decision, or whatever the attachment is filed against) resolves to a live row in the caller's own tenant before writing anything. That check has been there since the RPC shipped. It just never occurred to anyone to ask the same question about the file payload itself, because the existence check for the parent object and a tenant check for the storage path look like two different problems until you're reading the function end to end specifically looking for what it forgot to ask.

That's the same method the last several defect-audit passes have used: pick a surface nobody has read closely yet, and read the whole thing, not just the part that obviously matters. The attachment RPCs hadn't had a dedicated pass before this one. They have now — and pgTAP has a case pinning both the rejection (a foreign-tenant-prefixed path) and the still-works case (a correctly-scoped one), so the boundary this fix draws stays drawn.

Start at ledgenter.com.