Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions .changeset/25694-anonymous-write-carries-no-file.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
---
'@objectstack/service-storage': patch
---

fix(storage)!: an anonymous write carries no file

Ruling C ("an anonymous submission carries no file") now holds at every anonymous write door of ADR-0138 D2, not only at the public form door. A record write whose caller has no user, such as the guest an `authRequired: false` endpoint executes as under the grants a deployment binds to the guest, may not put a value in a file field (`file`, `image`, `avatar`, `video`, `audio`). It is refused before anything is written, with the form door's envelope: `400 VALIDATION_FAILED`, one `fields[]` entry per refused field carrying the ADR-0114 field code `invalid_value` and the field's label. An empty or absent file field is written as before. Signed-in callers, system writes and context-less programmatic writes are unchanged. No error code is added.

Clause-②: no (narrowing) — an anonymous write that carried a value in a file field is refused. Nothing that was refused becomes accepted.

Metadata-protocol: none (a write path's file rule; nothing a metadata author writes changes)

<!-- adr-0087: not-required (no-migration-prescription) No metadata moves: no spec key, authorable spelling, object definition, stored row or stored shape is added, removed, renamed or re-shaped, and no exported declaration is removed or narrowed, so there is nothing for `objectstack migrate meta` to rewrite. What changes is which record writes the file-reference path accepts (an anonymous caller's write carrying a file value is refused). The remedy is a change of who provides the file (a signed-in caller), never a rewrite of a package source or a stored row. The other categories are closed on facts: the package publishes (not unpublished); no ADR-0087 id is named or touched (not registered or already-registered); and nothing exported is removed or narrowed (not runtime-interface-only, type-surface-only or runtime-export-retired). -->

**BREAKING** for an anonymous write that carried a file value, shipped as `patch` because the metadata protocol does not move.

- **Before.** Only the public form door refused a file value from an anonymous caller.
- **Now.** Every write whose caller has no user is refused with `400 VALIDATION_FAILED` when a file field carries a value.
- **Fix.** Leave file fields empty in an anonymous write. A file is provided by a signed-in caller, through the authenticated upload doors.
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,10 @@ describe('sys_file public-scope backfill (#22443 ruling B) — a real ObjectQL o
* the source row's scope on the copy.
*/
// The claiming write acts in an organization: `product` is created by this
// release, whose tenant column is NOT NULL (ADR-0131 D1).
// release, whose tenant column is NOT NULL (ADR-0131 D1). [#25694] Its
// caller is signed in: an anonymous write carries no file.
const copyBySecondReference = (recordId: string, fileId: string) =>
engine.insert('product', { id: recordId, title: recordId, image: fileId }, { context: { tenantId: 'org_a' } } as any);
engine.insert('product', { id: recordId, title: recordId, image: fileId }, { context: { userId: 'u_writer', tenantId: 'org_a' } } as any);

it('a deployment with no public rows runs clean: it counts zero and writes nothing', async () => {
await engine.insert('sys_file', { ...legacyPublicRow('f_user'), scope: 'user' }, SYSTEM as any);
Expand Down
189 changes: 189 additions & 0 deletions packages/services/service-storage/src/file-reference-lifecycle.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
type FileReferenceEngine,
} from './file-reference-lifecycle.js';
import { assertEngineFindOnePredicate } from '@objectstack/objectql';
import { resolveThrownHttpError } from '@objectstack/types';

const silentLogger = () => ({ info: vi.fn(), warn: vi.fn(), debug: vi.fn() });

Expand Down Expand Up @@ -1357,3 +1358,191 @@ describe('a write references only a file its caller may read', () => {
});
});
});

describe('an anonymous write carries no file (#25694, ruling C at every anonymous write door)', () => {
/** The guest principal an `authRequired: false` endpoint's write executes as (ADR-0138 D9). */
const GUEST = { principalKind: 'guest', positions: ['guest'], isSystem: false, tenantId: 'org_1' };
const guestSession = { organizationId: 'org_1', positions: ['guest'] };
const SIGNED_IN = { userId: 'u_writer', tenantId: 'org_1' };

const LABELLED: Record<string, any> = {
sys_file: REGISTRY.sys_file,
product: {
name: 'product',
fields: {
id: { type: 'text' },
name: { type: 'text' },
image: { type: 'image', label: 'Photo' },
gallery: { type: 'image', multiple: true },
manual: { type: 'file' },
},
},
};

function installAdmitting(engine: Engine, storage: any = fakeStorage()) {
// A verdict that admits every file: the refusal must not lean on it.
const authorizeRead = vi.fn(async () => true);
installFileReferenceHooks(engine, () => storage, silentLogger(), authorizeRead);
return { storage, authorizeRead };
}

async function write(
engine: Engine,
phase: 'insert' | 'update',
data: Record<string, unknown>,
id: string,
caller: { session?: any; context?: any },
) {
const ctx: any = {
object: 'product',
event: phase === 'insert' ? 'beforeInsert' : 'beforeUpdate',
input: { ...(phase === 'update' ? { id } : {}), data, options: caller.context ? { context: caller.context } : {} },
session: caller.session,
dispatch: recordDispatch(),
};
await engine.trigger(ctx.event, ctx);
let row: Record<string, unknown> & { id: string };
if (phase === 'insert') {
row = { ...(ctx.input.data as Record<string, unknown>), id };
(engine.tables.product ??= []).push(row);
} else {
row = (engine.tables.product ?? []).find((r) => String(r.id) === id) as any;
Object.assign(row, ctx.input.data);
}
ctx.event = phase === 'insert' ? 'afterInsert' : 'afterUpdate';
ctx.result = row;
await engine.trigger(ctx.event, ctx);
return row;
}

const asGuest = { session: guestSession, context: GUEST };
const refusedBy = (p: Promise<unknown>) => p.then(() => null, (e) => e);
const sysFileWrites = (engine: Engine) =>
engine.calls.filter((c) => (c.op === 'update' || c.op === 'insert') && c.object === 'sys_file');

describe('a value in a file field is refused before anything is written', () => {
const VALUES: Array<[string, unknown]> = [
['an unclaimed file id', 'file_a'],
['a file id another record owns', 'file_b'],
['an external URL', 'https://example.test/x.png'],
['an inline file value', { url: 'https://example.test/y.png', name: 'y.png' }],
];
for (const [what, value] of VALUES) {
it(`on insert: ${what}`, async () => {
const engine = fakeEngine({
registry: LABELLED,
files: [file(), file({ id: 'file_b', key: 'user/file_b.png', ref_object: 'product', ref_id: 'p0', ref_field: 'image' })],
records: { product: [{ id: 'p0', image: 'file_b' }] },
});
const { storage, authorizeRead } = installAdmitting(engine);

const err = await refusedBy(write(engine, 'insert', { name: 'n', image: value }, 'p1', asGuest));

const answered = resolveThrownHttpError(err);
expect({ status: answered.status, code: answered.code }).toEqual({ status: 400, code: 'VALIDATION_FAILED' });
expect(err?.fields).toEqual([expect.objectContaining({ field: 'image', code: 'invalid_value', label: 'Photo' })]);
expect(err?.fields[0].message).toContain('Photo');
// Nothing was written: no row, no claim, no copy, and no file was looked up for it.
expect((engine.tables.product ?? []).map((r) => r.id)).toEqual(['p0']);
expect(sysFileWrites(engine)).toEqual([]);
expect(storage.download).not.toHaveBeenCalled();
expect(authorizeRead).not.toHaveBeenCalled();
});
}

it('on update', async () => {
const engine = fakeEngine({ registry: LABELLED, files: [file()], records: { product: [{ id: 'p1', image: null }] } });
installAdmitting(engine);

const err = await refusedBy(write(engine, 'update', { image: 'file_a' }, 'p1', asGuest));

expect(err?.code).toBe('VALIDATION_FAILED');
expect(err?.fields).toEqual([expect.objectContaining({ field: 'image', code: 'invalid_value' })]);
expect(engine.tables.product[0].image).toBeNull();
expect(engine.tables.sys_file[0].ref_id ?? null).toBeNull();
});

it('every file field carrying a value is named in one envelope, by its label or else its name', async () => {
const engine = fakeEngine({ registry: LABELLED, files: [file()] });
installAdmitting(engine);

const err = await refusedBy(
write(engine, 'insert', { image: 'file_a', gallery: ['file_a'], manual: 'https://example.test/m.pdf' }, 'p1', asGuest),
);

expect(err?.code).toBe('VALIDATION_FAILED');
expect(err?.fields.map((f: any) => [f.field, f.code, f.label])).toEqual([
['image', 'invalid_value', 'Photo'],
['gallery', 'invalid_value', 'gallery'],
['manual', 'invalid_value', 'manual'],
]);
});
});

describe('CONTROL — what the rule leaves as it was', () => {
it('an anonymous write with every file field empty or absent is written', async () => {
const engine = fakeEngine({ registry: LABELLED, files: [file()] });
installAdmitting(engine);

for (const [id, data] of [
['p1', { name: 'absent' }],
['p2', { name: 'null', image: null }],
['p3', { name: 'blank', image: '' }],
['p4', { name: 'empty set', gallery: [] }],
] as const) {
await write(engine, 'insert', { ...data }, id, asGuest);
}

expect(engine.tables.product.map((r) => r.id)).toEqual(['p1', 'p2', 'p3', 'p4']);
expect(engine.tables.sys_file[0].ref_id ?? null).toBeNull();
});

it('an anonymous update that clears a file field, or leaves it out, is written', async () => {
const engine = fakeEngine({ registry: LABELLED, records: { product: [{ id: 'p1', name: 'a', image: null }] } });
installAdmitting(engine);

await write(engine, 'update', { name: 'b' }, 'p1', asGuest);
await write(engine, 'update', { image: null }, 'p1', asGuest);

expect(engine.tables.product[0]).toMatchObject({ name: 'b', image: null });
});

it('a signed-in caller\'s write still claims the file', async () => {
const engine = fakeEngine({ registry: LABELLED, files: [file()] });
installAdmitting(engine);

await write(engine, 'insert', { image: 'file_a' }, 'p1', {
session: { userId: SIGNED_IN.userId, organizationId: 'org_1' },
context: SIGNED_IN,
});

expect(engine.tables.sys_file[0]).toMatchObject({ ref_object: 'product', ref_id: 'p1', ref_field: 'image' });
});

it('a system write and a context-less write still claim the file', async () => {
const engine = fakeEngine({ registry: LABELLED, files: [file(), file({ id: 'file_c', key: 'user/file_c.png' })] });
installAdmitting(engine);

await write(engine, 'insert', { image: 'file_a' }, 'p1', { session: { isSystem: true }, context: { isSystem: true } });
await write(engine, 'insert', { image: 'file_c' }, 'p2', {});

expect(engine.tables.sys_file.map((f) => [f.id, f.ref_id ?? null])).toEqual([
['file_a', 'p1'],
['file_c', 'p2'],
]);
});

it('an object with no file field is not judged', async () => {
const engine = fakeEngine({ files: [file()] });
installAdmitting(engine);
const ctx: any = {
object: 'tag', event: 'beforeInsert', input: { data: { label: 'file_a' }, options: { context: GUEST } },
session: guestSession, dispatch: recordDispatch(),
};

await engine.trigger('beforeInsert', ctx);

expect(engine.calls).toEqual([]);
});
});
});
68 changes: 66 additions & 2 deletions packages/services/service-storage/src/file-reference-lifecycle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import { randomUUID } from 'node:crypto';
import { FILE_REFERENCE_TYPES, isFileIdToken, RAW_FILE_VALUES_CONTEXT_KEY } from '@objectstack/spec/data';
import type { IStorageService } from '@objectstack/spec/contracts';
import type { ExecutionContext } from '@objectstack/spec/kernel';
import { isEmptyForRequired } from '@objectstack/core';
import { validationFailure } from '@objectstack/types';
import { callerContext } from './attachment-access-hooks.js';

/**
Expand Down Expand Up @@ -51,6 +53,18 @@ import { callerContext } from './attachment-access-hooks.js';
* the read check. System writes and context-less programmatic calls carry no
* caller to ask, and reference files as before.
*
* ### An anonymous write carries no file
*
* [#25694] Ruling C (record 6384525042), held at every anonymous write door
* rather than at the public form door alone: a write whose caller has no user
* (ADR-0138 D2's anonymous write doors: the guest envelope of an
* `authRequired: false` endpoint, under whatever grants a deployment binds to
* the guest) may not put a value in a file field. It is refused before the driver runs, naming each
* field, with the form door's envelope ({@link assertAnonymousWriteCarriesNoFile}).
* The read verdict above would admit a `public_read` file to such a caller,
* and keep any other id as written; neither is a file the anonymous caller
* may provide. An empty or absent file field is written as before.
*
* ### The one write that cannot honour it, and is refused
*
* [#7102] A predicate (`multi: true`) update has ONE payload for N matched
Expand Down Expand Up @@ -277,6 +291,50 @@ async function findReferenceableFile(
return (await authorizeRead(row, caller)) ? row : null;
}

/**
* [#25694] Ruling C at every anonymous write door: an anonymous write carries
* no file. A file is provided by a signed-in caller, through the authenticated
* upload doors; a caller with no user has none to provide, so a value in a
* file field of its write is refused, whether it names a platform file (one it
* may download or not) or anything else. Judged before the driver runs and
* before any file is looked up, so a refused write leaves nothing behind.
*
* The form door's own judgement (`form-routes.ts`, #25572), held at the
* engine: the same emptiness rule (`isEmptyForRequired`), and the same
* envelope — one `400 VALIDATION_FAILED` naming every refused field by its
* label with the closed ADR-0114 field code `invalid_value`. The form door
* keeps its own judgement, which answers before anything reaches the engine.
*
* `caller` is {@link referencingCallerOf}'s answer, the caller the read
* verdict is asked about: a system write and a context-less programmatic
* write have none, and are not judged; a caller with no user is anonymous.
*/
function assertAnonymousWriteCarriesNoFile(
engine: FileReferenceEngine,
object: string,
data: Record<string, unknown>,
fileFields: string[],
caller: ExecutionContext | null,
): void {
if (!caller || caller.userId) return;
const refused: Array<{ field: string; code: 'invalid_value'; message: string; label: string }> = [];
for (const field of fileFields) {
if (!Object.prototype.hasOwnProperty.call(data, field)) continue;
const def = fieldDefOf(engine, object, field);
if (isEmptyForRequired(def, data[field])) continue;
const declared = typeof def?.label === 'string' ? def.label.trim() : '';
const label = declared.length > 0 ? declared : field;
refused.push({
field,
code: 'invalid_value',
message: `${label} cannot take a file here: an anonymous write carries no file. `
+ 'Leave it empty, or sign in and provide the file from your account.',
label,
});
}
if (refused.length > 0) throw validationFailure(refused.map((f) => f.message).join('; '), refused);
}

/** Bound on owned files released per record delete. */
const RELEASE_BATCH_LIMIT = 1_000;

Expand Down Expand Up @@ -869,9 +927,11 @@ export function installFileReferenceHooks(
if (!object || !data || typeof data !== 'object') return;
const fileFields = activeFileFields(engine, object);
if (fileFields.length === 0) return;
const caller = referencingCallerOf(ctx);
assertAnonymousWriteCarriesNoFile(engine, object, data, fileFields, caller);
await applyCopyOnClaim(
engine, getStorage, logger, object, null, data, fileFields,
actingOrganizationOf(ctx), referencingCallerOf(ctx), authorizeRead, mintedCopiesFor(ctx),
actingOrganizationOf(ctx), caller, authorizeRead, mintedCopiesFor(ctx),
);
},
{ packageId: PACKAGE_ID },
Expand All @@ -885,6 +945,10 @@ export function installFileReferenceHooks(
if (!object || !data || typeof data !== 'object') return;
const fileFields = activeFileFields(engine, object);
if (fileFields.length === 0) return;
// [#25694] Before the predicate refusal below: an anonymous write is
// refused for what it carries, whichever shape the write takes.
const caller = referencingCallerOf(ctx);
assertAnonymousWriteCarriesNoFile(engine, object, data, fileFields, caller);
// [#6966] "Is this a multi-update?" is the engine's marker, not the shape
// of `input.id`. A per-row context names ITS row, so the old
// `ids.length === 1` test answered "single update" on every row of a
Expand Down Expand Up @@ -928,7 +992,7 @@ export function installFileReferenceHooks(
// analysis a later edit has to re-derive.
await applyCopyOnClaim(
engine, getStorage, logger, object, recordId, data, fileFields,
actingOrganizationOf(ctx), referencingCallerOf(ctx), authorizeRead, mintedCopiesFor(ctx),
actingOrganizationOf(ctx), caller, authorizeRead, mintedCopiesFor(ctx),
);
},
{ packageId: PACKAGE_ID },
Expand Down
Loading
Loading