Security: attachment blob download has no record-level RLS — bytes served by fileId with only a committed-status check (IDOR)

[os-zhuang]

Surfaced by the ADR-0096 E4 execution-surface verification (the one live unaudited surface). Attachment blob bytes are served under a bearer-capability model keyed only by sys_file.id — there is no record-level RLS re-check tying the download to the caller's ability to read the owning sys_attachment row or its parent record.

The hole

GET /api/v1/storage/files/:fileId — packages/services/service-storage/src/storage-routes.ts:409-432 (and the JSON twin /files/:fileId/url, :373-396):

• The only gate is file.status === 'committed'. No caller identity, no ExecutionContext, no sys_attachment lookup, no parent-record visibility check.
• store.getFile(fileId) (metadata-store.ts:82-92) uses engine.findOne('sys_file', { where: { id } }). IDataEngine.findOne (packages/spec/src/contracts/data-engine.ts:30) takes no ExecutionContext — it's the raw system-level engine, so the lookup is inherently un-RLS'd.

Signed URLs are not caller-scoped either. LocalStorageAdapter.getPresignedDownload (local-storage-adapter.ts:271-278) signs a token whose payload (:48-53) is { k: key, exp, op } — storage key + expiry only, no user/tenant binding. The redeem route GET /api/v1/storage/_local/raw/:token (storage-routes.ts:468-487) verifies only HMAC + expiry, then streams raw bytes. Pure bearer capability; minting is gated by nothing more than knowing the fileId.

The parent-record link is bypassed entirely. sys_attachment (packages/platform-objects/src/audit/sys-attachment.object.ts) is the RLS-bearing polymorphic link (parent_object/parent_id/visibility/share_type) and it rides normal CRUD/RLS — but nothing in the download path consults it. Revoking a sys_attachment share does not revoke access to the bytes.

Repro

1. User A uploads a file (fileId = v4 UUID) and attaches it to record R via a sys_attachment row. R (and thus the attachment row) is invisible to User B under RLS.
2. B obtains the fileId via any channel that surfaces file_id without gating the blob (a later-revoked grant, an API/audit payload, a forwarded link, a denormalized field).
3. GET /api/v1/storage/files/<fileId> → 302 → signed /_local/raw/<token> → full file bytes, with zero re-check that B may read the attachment or its parent. Access survives revocation of the share.

Severity nuance

• fileId is a v4 UUID → not brute-force enumerable. This is a capability-leak / IDOR-on-known-id, not blind enumeration — lowers likelihood, does not close the hole (the byte-access model is "possession of the id," not RLS).
• Open sub-question (needs verification): are /api/v1/storage/* routes behind global authentication? They're registered directly on IHttpServer in storage-service-plugin.ts:204, not through RestServer.enforceAuth (which only guards /api/v1/data). If storage routes are outside the auth middleware, the exposure widens to unauthenticated callers. Either way, even a fully authenticated caller gets no record-level authorization on the bytes.

Fix direction

Before minting/redirecting, the download handlers (storage-routes.ts:373, :409) must resolve the caller's ExecutionContext and load the owning sys_attachment (and/or sys_file) through the RLS-enforcing object-CRUD layer (not raw engine.findOne), returning 404 if not visible; and/or bind the signed token to the caller identity so /_local/raw/:token cannot be replayed cross-user. Resolve the auth-gate sub-question as part of this.

Relationship to ADR-0096

E2/E3-class instance found by the E4 sweep: a surface serving data with no caller-scoped authorization. Exactly what the ADR-0096 D4 conformance matrix (a caller-scoped? proof for the storage surface) would have required. Refs: ADR-0096 (#2975 / commit e07645c), #2849, #2980 (the reports analogue).

[os-zhuang]

Correction — the core IDOR is already fixed on current main (#2970); this issue was filed against stale code

While starting the fix I read the current code and found the E4 verification had run against a pre-#2970 snapshot. On main today (929efdf), both download routes are RLS-gated. Retracting the P1 framing:

What's actually enforced now (storage-service-plugin.ts:442-496 buildFileReadAuthorizer, wired at :257; gate at storage-routes.ts:88-114, applied by BOTH /files/:fileId :530 and /files/:fileId/url :491):

• For scope === 'attachments', non-public_read files: resolves the caller's RLS context from the request; no session → 401 (unauthenticated); owner → allow; otherwise loads the sys_attachment links and allows only if the caller can see a parent record under their OWN RLS (context: authz), else 403; fail-closed on any error.
• The check is live per download, so revoking the sys_attachment share now does revoke byte access — contradicting my "survives revocation" claim.

So the "any authenticated user reads any attachment by fileId" IDOR is closed for attachment files. My original report reflected the older path that served bytes on a bare status==='committed' check.

Genuine residuals (smaller, partly by-design) — the only things left to decide:

1. Non-attachments-scope files (avatars, org logos, scope:'user') keep a stable anonymous capability URL — authorizeDownload early-returns for scope !== 'attachments' (storage-routes.ts:93). This is a deliberate [attachments] v2 tracking: parent-visibility inheritance, authed downloads, remaining enforce-or-remove #2970 decision (these are embedded in <img src>/<a href> which can't carry a bearer token) and is already flagged as a tracked follow-up in the code (:26-27). Whether record field-files that are not attachments-scope need gating is a threat-model call — not the reported IDOR.
2. Signed tokens aren't caller-bound ({k,exp,op}) — but they're now only minted after the authorization passes and are short-lived (downloadTtl 300s). A leaked token is replayable only within its TTL. Minor.

Recommendation: close this as resolved by #2970, or relabel to P3 and narrow the title to residual #1 (non-attachments-scope anonymous capability URLs) if the team wants that hardened. Either way it is not a live P1. I'll leave the state change to a maintainer.

(Filed from the ADR-0096 E4 sweep; corrected after reading current main. The E4 finding stands as "verified — already remediated," which is itself a good D4 outcome: the storage surface has a caller-scoped download proof.)

---

Generated by Claude Code