From dc0a938662e7f49b8486a0a33f326fedc28b21a0 Mon Sep 17 00:00:00 2001 From: Liam Potter Date: Thu, 6 Aug 2026 22:12:09 +0100 Subject: [PATCH 1/2] Gate every relationship path on exposeRelationships isRelationExposed was documented as the single home for the relation-visibility rule, but two write paths never went through it. The relationship endpoints used getRelationOrFail, which honoured serializeAs and ignored exposeRelationships, and the deserializer had its own inline serializeAs check, so both a relationships controller and a resource POST or PATCH body could reach a hidden relation. getRelationOrFail now takes the registry and applies the rule. The parameter is required rather than optional so a new call site cannot skip the check, which is how the drift happened. The deserializer consults the same rule for relationship members. A hidden relation is indistinguishable from one that does not exist on every path: 404 on the endpoints, where the name is a URL segment, and the unknown-member 400 in write bodies. A 403 would confirm the relation exists. Closes #10 --- docs/reading-data.md | 14 + docs/writing-data.md | 4 + src/context.ts | 2 +- src/deserializer.ts | 5 +- src/relationships.ts | 19 +- tests/unit/kebab_case.spec.ts | 8 +- .../relationship_endpoint_exposure.spec.ts | 246 ++++++++++++++++++ 7 files changed, 288 insertions(+), 10 deletions(-) create mode 100644 tests/unit/relationship_endpoint_exposure.spec.ts diff --git a/docs/reading-data.md b/docs/reading-data.md index 4f5b6e6..5dde6f7 100644 --- a/docs/reading-data.md +++ b/docs/reading-data.md @@ -121,6 +121,20 @@ static exposeRelationships = ['author', 'tags'] Hidden relations are hidden from `?include=` too. Asking to include one is rejected with a 400, exactly like an include path that does not exist, and no preloading happens for it. This keeps a deliberately hidden relation from being loaded (and paid for) just to be discarded at serialization. +A hidden relation is also unreachable through the relationship endpoints, for reads and writes both: + +``` +GET /articles/1/comments 404 +GET /articles/1/relationships/comments 404 +PATCH /articles/1/relationships/comments 404 +POST /articles/1/relationships/comments 404 +DELETE /articles/1/relationships/comments 404 +``` + +The status is 404 rather than 403, so a hidden relation cannot be told apart from one that was never defined. A 403 would confirm the relation exists, which is the thing you were hiding. + +The same applies to the `relationships` member of a `POST` or `PATCH` body. A hidden relation there is rejected with the same 400 an unknown member gets. Hiding a relation removes it from the API everywhere: documents, `?include=`, the relationship endpoints, and write bodies. + ### `static filters` Declares the `?filter[...]` parameters this resource accepts. Nothing is filterable without it. Covered in depth in [Filtering](#filtering) below. diff --git a/docs/writing-data.md b/docs/writing-data.md index ae7cfa3..e78754a 100644 --- a/docs/writing-data.md +++ b/docs/writing-data.md @@ -58,6 +58,8 @@ The deserializer enforces the spec's error semantics for you: Attribute names are mapped back from their serialized names to model property names. Unknown attributes are dropped, and your validator remains the gatekeeper. +A relation hidden by [`exposeRelationships`](./reading-data.md#static-exposerelationships) counts as unknown here. A `relationships` member naming one is rejected with the same `400`, so hiding a relation closes the resource-body write path too. + ## Relationship endpoints The spec defines URLs for reading and editing a relationship itself, without touching the resources on either end. Editing linkage through these URLs sends deltas rather than snapshots, which protects concurrent editors from overwriting each other; the [links guide](./links.md) walks through a lost-update example. `jsonApiResource` registers the endpoints when you provide a `relationships` controller: @@ -88,6 +90,8 @@ export default class ArticleRelationshipsController { To-one relationships accept `PATCH` only (a `405` otherwise). For `hasMany`, full replacement and removal are rejected with `403`. The spec explicitly allows a server to refuse those, and the natural write path for a hasMany is the child's own belongsTo. `manyToMany` supports everything. `hasManyThrough` relationships are derived, and all writes through them are rejected. +All five routes respect the resource's [`exposeRelationships`](./reading-data.md#static-exposerelationships). A relation the resource does not expose returns `404` here as well, so registering this controller cannot reopen something the resource hides. + --- Next: [Links](./links.md) · [Errors & negotiation](./errors.md) · [Reference](./reference.md) diff --git a/src/context.ts b/src/context.ts index bd65881..3da3d69 100644 --- a/src/context.ts +++ b/src/context.ts @@ -242,7 +242,7 @@ export class JsonApiRequestContext { */ async renderRelated(row: LucidRow, name: string): Promise { const Model = row.constructor as LucidModel - const relation = getRelationOrFail(Model, name) + const relation = getRelationOrFail(Model, name, this.#registry) const relationName = relation.relationName await loadRelation(row, relationName) const loaded = row.$preloaded[relationName] as LucidRow | LucidRow[] | null | undefined diff --git a/src/deserializer.ts b/src/deserializer.ts index 0dde8b4..6c85c83 100644 --- a/src/deserializer.ts +++ b/src/deserializer.ts @@ -1,5 +1,6 @@ import type { LucidModel } from '@adonisjs/lucid/types/model' import { JsonApiException } from './errors.ts' +import { isRelationExposed } from './resource.ts' import type { ResourceIdentifier } from './types.ts' import type { JsonApiRegistry } from './registry.ts' @@ -174,7 +175,9 @@ function deserializeRelationships( for (const [name, value] of Object.entries(raw)) { const pointer = `/data/relationships/${name}` const relation = Model.$relationsDefinitions.get(name) - if (!relation || relation.serializeAs === null) { + // isRelationExposed covers serializeAs: null as well, and a hidden + // relation reads the same as an unknown one on purpose. + if (!relation || !isRelationExposed(registry.resourceFor(Model), name, relation)) { throw invalidDocument(`"${name}" is not a known relationship of ${Model.name}`, pointer) } if (!isPlainObject(value) || !('data' in value)) { diff --git a/src/relationships.ts b/src/relationships.ts index 80e9758..50aadb1 100644 --- a/src/relationships.ts +++ b/src/relationships.ts @@ -3,6 +3,7 @@ import type { LucidModel, LucidRow } from '@adonisjs/lucid/types/model' import { loadRelation, relatedClient, setAttribute } from './lucid_access.ts' import { JsonApiException } from './errors.ts' import { verifyRelatedExist } from './deserializer.ts' +import { isRelationExposed } from './resource.ts' import type { JsonApiRegistry } from './registry.ts' import type { ResourceIdentifier } from './types.ts' @@ -23,11 +24,17 @@ function invalidLinkage(detail: string): JsonApiException { * Resolves a relationship by name, accepting both the Lucid relation name * and its kebab-cased URL segment (received-comments → receivedComments). * Returns the booted relation; use relation.relationName for Lucid calls. + * + * A relation the resource does not expose is reported as missing rather than + * forbidden, so a hidden relation cannot be told apart from one that was + * never defined. The registry is required so a new call site cannot reach a + * relation without passing the visibility rule. */ -export function getRelationOrFail(Model: LucidModel, name: string) { - const relation = - Model.$relationsDefinitions.get(name) ?? Model.$relationsDefinitions.get(string.camelCase(name)) - if (!relation || relation.serializeAs === null) { +export function getRelationOrFail(Model: LucidModel, name: string, registry: JsonApiRegistry) { + const key = Model.$relationsDefinitions.has(name) ? name : string.camelCase(name) + const relation = Model.$relationsDefinitions.get(key) + + if (!relation || !isRelationExposed(registry.resourceFor(Model), key, relation)) { throw new JsonApiException( { title: 'Not Found', detail: `"${name}" is not a relationship of ${Model.name}` }, { status: 404 } @@ -90,7 +97,7 @@ export async function updateRelationship( action: RelationshipAction ): Promise { const Model = row.constructor as LucidModel - const relation = getRelationOrFail(Model, name) + const relation = getRelationOrFail(Model, name, registry) const relationName = relation.relationName const relatedType = registry.typeFor(relation.relatedModel()) @@ -165,7 +172,7 @@ export async function fetchLinkage( registry: JsonApiRegistry ): Promise { const Model = row.constructor as LucidModel - const relation = getRelationOrFail(Model, name) + const relation = getRelationOrFail(Model, name, registry) const relationName = relation.relationName await loadRelation(row, relationName) const loaded = row.$preloaded[relationName] as LucidRow | LucidRow[] | null | undefined diff --git a/tests/unit/kebab_case.spec.ts b/tests/unit/kebab_case.spec.ts index 5d1b548..3b0aee5 100644 --- a/tests/unit/kebab_case.spec.ts +++ b/tests/unit/kebab_case.spec.ts @@ -37,10 +37,14 @@ test.group('kebab-case types and URLs', () => { }) test('kebab URL segments resolve back to Lucid relation names', ({ assert }) => { - const relation = getRelationOrFail(User, 'received-comments') + const registry = new JsonApiRegistry() + const relation = getRelationOrFail(User, 'received-comments', registry) assert.equal(relation.relationName, 'receivedComments') // the Lucid name itself still works too - assert.equal(getRelationOrFail(User, 'receivedComments').relationName, 'receivedComments') + assert.equal( + getRelationOrFail(User, 'receivedComments', registry).relationName, + 'receivedComments' + ) }) test('routes-strategy links pass the kebab segment as the route param', ({ assert }) => { diff --git a/tests/unit/relationship_endpoint_exposure.spec.ts b/tests/unit/relationship_endpoint_exposure.spec.ts new file mode 100644 index 0000000..43ad1da --- /dev/null +++ b/tests/unit/relationship_endpoint_exposure.spec.ts @@ -0,0 +1,246 @@ +/** + * A relation left out of exposeRelationships must be unreachable through the + * relationship endpoints, not just absent from documents and rejected in + * ?include=. Serialization and include validation already share the rule via + * isRelationExposed; these tests pin the endpoints to the same rule. + * + * The gate lives in getRelationOrFail, which runs before any database access, + * so fetchLinkage and updateRelationship can be tested against unsaved rows. + */ +import { test } from '@japa/runner' +import { HttpContextFactory } from '@adonisjs/core/factories/http' +import { JsonApiRegistry } from '../../src/registry.ts' +import { JsonApiResource } from '../../src/resource.ts' +import { JsonApiException } from '../../src/errors.ts' +import { JsonApiRequestContext } from '../../src/context.ts' +import { defineConfig } from '../../src/define_config.ts' +import { deserializeResourceDocument } from '../../src/deserializer.ts' +import { fetchLinkage, getRelationOrFail, updateRelationship } from '../../src/relationships.ts' +import { Article, make } from '../fixtures/models.ts' + +/** + * Registry where Article exposes only the named relations, hiding the rest. + */ +function registryHiding(...exposed: string[]) { + class ArticleResource extends JsonApiResource
{ + static model = () => Article + static exposeRelationships = exposed + } + return new JsonApiRegistry().register([ArticleResource]) +} + +/** + * Runs an async call expected to reject and hands back the exception. + * JsonApiException carries its title as the message, so the detail and status + * have to be read off the object rather than matched against a message string. + */ +async function rejection(fn: () => Promise): Promise { + try { + await fn() + } catch (error) { + return error as JsonApiException + } + throw new Error('expected the call to reject, it resolved') +} + +test.group('getRelationOrFail and exposeRelationships', () => { + test('a relation left out of exposeRelationships is a 404', ({ assert }) => { + const registry = registryHiding('tags') + + const error = assert.throws( + () => getRelationOrFail(Article, 'author', registry), + JsonApiException + ) as unknown as JsonApiException + + assert.equal(error.status, 404) + assert.match(error.errors[0].detail!, /"author" is not a relationship of Article/) + }) + + test('an unexposed relation is indistinguishable from one that does not exist', ({ assert }) => { + const registry = registryHiding('tags') + + const hidden = assert.throws( + () => getRelationOrFail(Article, 'author', registry), + JsonApiException + ) as unknown as JsonApiException + const unknown = assert.throws( + () => getRelationOrFail(Article, 'nonsense', registry), + JsonApiException + ) as unknown as JsonApiException + + assert.equal(hidden.status, unknown.status) + assert.equal(hidden.errors[0].title, unknown.errors[0].title) + }) + + test('an exposed relation is returned', ({ assert }) => { + const registry = registryHiding('tags') + const relation = getRelationOrFail(Article, 'tags', registry) + assert.equal(relation.relationName, 'tags') + }) + + test('every relation stays reachable when the resource sets no exposeRelationships', ({ + assert, + }) => { + class ArticleResource extends JsonApiResource
{ + static model = () => Article + } + const registry = new JsonApiRegistry().register([ArticleResource]) + + for (const name of ['author', 'comments', 'tags']) { + assert.equal(getRelationOrFail(Article, name, registry).relationName, name) + } + }) + + test('an unregistered model keeps every relation reachable', ({ assert }) => { + const registry = new JsonApiRegistry() + assert.equal(getRelationOrFail(Article, 'author', registry).relationName, 'author') + }) + + test('an unknown relation is still a 404', ({ assert }) => { + const registry = registryHiding('tags') + const error = assert.throws( + () => getRelationOrFail(Article, 'nonsense', registry), + JsonApiException + ) as unknown as JsonApiException + assert.equal(error.status, 404) + }) +}) + +test.group('relationship endpoints honour exposeRelationships', () => { + test('GET /:id/relationships/:name rejects an unexposed relation', async ({ assert }) => { + const registry = registryHiding('tags') + const article = make(Article, { title: 'T', authorId: 7 }) + + const error = await rejection(() => fetchLinkage(article, 'author', registry)) + assert.equal(error.status, 404) + assert.match(error.errors[0].detail!, /"author" is not a relationship of Article/) + }) + + test('PATCH /:id/relationships/:name rejects an unexposed to-one relation', async ({ + assert, + }) => { + const registry = registryHiding('tags') + const article = make(Article, { title: 'T', authorId: 7 }) + + const error = await rejection(() => + updateRelationship( + article, + 'author', + registry, + { data: { type: 'users', id: '1' } }, + 'replace' + ) + ) + assert.equal(error.status, 404) + assert.match(error.errors[0].detail!, /"author" is not a relationship of Article/) + }) + + test('GET /:id/:name rejects an unexposed relation', async ({ assert }) => { + const registry = registryHiding('tags') + const article = make(Article, { title: 'T', authorId: 7 }) + const jsonApi = new JsonApiRequestContext( + new HttpContextFactory().create(), + registry, + defineConfig({}) + ) + + const error = await rejection(() => jsonApi.renderRelated(article, 'author')) + assert.equal(error.status, 404) + assert.match(error.errors[0].detail!, /"author" is not a relationship of Article/) + }) + + /** + * tags is a manyToMany, which supports all three write actions, so a + * rejection here is the exposure gate rather than the 403 hasMany returns + * for replace and remove. + */ + for (const action of ['replace', 'add', 'remove'] as const) { + test(`a ${action} write rejects an unexposed to-many relation`, async ({ assert }) => { + const registry = registryHiding('author') + const article = make(Article, { title: 'T', authorId: 7 }) + + const error = await rejection(() => + updateRelationship(article, 'tags', registry, { data: [] }, action) + ) + assert.equal(error.status, 404) + assert.match(error.errors[0].detail!, /"tags" is not a relationship of Article/) + }) + } +}) + +test.group('resource write bodies honour exposeRelationships', () => { + /** + * The relationships member of a POST or PATCH body is the third write + * path, alongside the relationship endpoints. A hidden relation must be + * rejected here too, with the same 400 an unknown member gets, so a + * hidden relation cannot be told apart from one that does not exist. + */ + test('a hidden relation in a POST body is rejected like an unknown one', ({ assert }) => { + const registry = registryHiding('tags') + + const error = assert.throws( + () => + deserializeResourceDocument(Article, registry, { + data: { + type: 'articles', + attributes: { title: 'T' }, + relationships: { author: { data: { type: 'users', id: '42' } } }, + }, + }), + JsonApiException + ) as unknown as JsonApiException + + assert.equal(error.status, 400) + assert.match(error.errors[0].detail!, /"author" is not a known relationship of Article/) + }) + + test('a hidden relation and an unknown one are indistinguishable', ({ assert }) => { + const registry = registryHiding('tags') + const bodyWith = (name: string) => ({ + data: { + type: 'articles', + relationships: { [name]: { data: null } }, + }, + }) + + const hidden = assert.throws( + () => deserializeResourceDocument(Article, registry, bodyWith('author')), + JsonApiException + ) as unknown as JsonApiException + const unknown = assert.throws( + () => deserializeResourceDocument(Article, registry, bodyWith('nonsense')), + JsonApiException + ) as unknown as JsonApiException + + assert.equal(hidden.status, unknown.status) + assert.equal(hidden.errors[0].title, unknown.errors[0].title) + }) + + test('an exposed relation in a POST body still deserializes', ({ assert }) => { + const registry = registryHiding('author') + + const result = deserializeResourceDocument(Article, registry, { + data: { + type: 'articles', + attributes: { title: 'T' }, + relationships: { author: { data: { type: 'users', id: '42' } } }, + }, + }) + assert.equal(result.attributes.authorId, '42') + }) + + test('a resource without exposeRelationships accepts every relation member', ({ assert }) => { + class ArticleResource extends JsonApiResource
{ + static model = () => Article + } + const registry = new JsonApiRegistry().register([ArticleResource]) + + const result = deserializeResourceDocument(Article, registry, { + data: { + type: 'articles', + relationships: { author: { data: { type: 'users', id: '7' } } }, + }, + }) + assert.equal(result.attributes.authorId, '7') + }) +}) From c6f69cc32bef323c4a794fb74903f5acc95031ae Mon Sep 17 00:00:00 2001 From: Liam Potter Date: Thu, 6 Aug 2026 22:12:09 +0100 Subject: [PATCH 2/2] Stop prettier reformatting generated files pnpm owns the lockfile and release-plan owns the changelog. Formatting either buries real changes in noise. --- .prettierignore | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .prettierignore diff --git a/.prettierignore b/.prettierignore new file mode 100644 index 0000000..44ed83f --- /dev/null +++ b/.prettierignore @@ -0,0 +1,5 @@ +# Generated files. Prettier rewrites both, which buries real changes in +# reformatting noise. pnpm owns the lockfile and release-plan owns the +# changelog, so neither should be hand-formatted. +pnpm-lock.yaml +CHANGELOG.md