Skip to content

Reject non-JSON-LD PUT on .acl resources (#295) #339

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
melvincarvalho merged 10 commits into from
Apr 30, 2026
Merged

Reject non-JSON-LD PUT on .acl resources (#295) #339

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
ae8316c
Reject non-JSON-LD PUT on .acl resources (#295)
melvincarvalho Apr 30, 2026
fd411e3
Address Copilot review on #295
melvincarvalho Apr 30, 2026
388f31e
Address Copilot round-2 review on #295
melvincarvalho Apr 30, 2026
67571c6
Address Copilot round-3 review on #295
melvincarvalho Apr 30, 2026
04b22ee
Address Copilot round-4 review on #295
melvincarvalho Apr 30, 2026
9aca012
Address Copilot round-5 review on #295
melvincarvalho Apr 30, 2026
dab7b0f
Address Copilot round-6 review on #295
melvincarvalho Apr 30, 2026
c36885a
Address Copilot round-7 review on #295
melvincarvalho Apr 30, 2026
01415d8
Address Copilot round-8 review on #295
melvincarvalho Apr 30, 2026
03de3cc
Address Copilot round-9 review on #295
melvincarvalho Apr 30, 2026
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
9 changes: 7 additions & 2 deletions src/handlers/container.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,16 @@ export async function handlePost(request, reply) {

// Check if we can accept this input type
if (!canAcceptInput(contentType, connegEnabled)) {
const acceptValue = connegEnabled
? 'application/ld+json, application/json, text/turtle, text/n3'
: 'application/ld+json, application/json';
reply.header('Accept', acceptValue);
reply.header('Accept-Post', acceptValue);
return reply.code(415).send({
error: 'Unsupported Media Type',
message: connegEnabled
? 'Supported types: application/ld+json, text/turtle, text/n3'
: 'Supported type: application/ld+json (enable conneg for Turtle support)'
? 'Supported types: application/ld+json, application/json, text/turtle, text/n3'
: 'Supported types: application/ld+json, application/json (enable conneg for Turtle/N3 support)'
});
}

Expand Down
26 changes: 24 additions & 2 deletions src/handlers/resource.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -614,13 +614,35 @@ export async function handlePut(request, reply) {

const contentType = request.headers['content-type'] || '';

// ACL resources require a JSON-LD payload (application/ld+json or
// application/json). Round-trip serialization between JSON-LD and
// Turtle representations has limitations that can cause data loss
// when a client PUTs Turtle and later requests Turtle.
// Other RDF resources are unaffected. The guard fires regardless
// of conneg setting and also when Content-Type is missing.
const ctMain = contentType.split(';')[0].trim().toLowerCase();
const isJsonLd = ctMain === 'application/ld+json' || ctMain === 'application/json';
if (urlPath.endsWith('.acl') && !isJsonLd) {
reply.header('Accept', 'application/ld+json, application/json');
reply.header('Accept-Put', 'application/ld+json, application/json');
return reply.code(415).send({
error: 'Unsupported Media Type',
message: 'ACL resources must be sent as application/ld+json or application/json.'
});
Comment on lines +626 to +631

Copilot AI Apr 30, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor inconsistency: this branch treats application/json as an allowed JSON-LD payload (and advertises it), but the later generic canAcceptInput() 415 error message in this handler still only lists application/ld+json as supported. Consider updating that other 415 message to also mention application/json so clients get accurate guidance across both paths (canAcceptInput already accepts application/json).

Copilot uses AI. Check for mistakes.
}

// Check if we can accept this input type
if (!canAcceptInput(contentType, connegEnabled)) {
const acceptValue = connegEnabled
? 'application/ld+json, application/json, text/turtle, text/n3'
: 'application/ld+json, application/json';
reply.header('Accept', acceptValue);
reply.header('Accept-Put', acceptValue);
return reply.code(415).send({
error: 'Unsupported Media Type',
message: connegEnabled
? 'Supported types: application/ld+json, text/turtle, text/n3'
: 'Supported type: application/ld+json (enable conneg for Turtle support)'
? 'Supported types: application/ld+json, application/json, text/turtle, text/n3'
: 'Supported types: application/ld+json, application/json (enable conneg for Turtle/N3 support)'
});
Comment on lines 634 to 646

Copilot AI Apr 30, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR description says 415 responses should include Accept and Accept-Put headers indicating supported types, but the canAcceptInput() 415 branch returns without setting those headers (only the .acl guard does). If that behavior is intended for all PUT 415s, consider adding the same Accept / Accept-Put headers here so clients can discover the required media types from the response.

Copilot uses AI. Check for mistakes.
}

Expand Down
23 changes: 18 additions & 5 deletions src/rdf/conneg.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,20 +205,33 @@ export function getVaryHeader(connegEnabled, mashlibEnabled = false) {
}

/**
* Get Accept-* headers for responses
* Get Accept-* headers for responses.
*
* The explicitly listed RDF types are aligned with the formats this
* module accepts so clients can discover support consistently:
* - JSON-LD (application/ld+json) and JSON (application/json alias)
* are advertised in all conneg modes.
* - Turtle (text/turtle) and N3 (text/n3) are advertised only when
* conneg is enabled (SUPPORTED_INPUT in this file).
*
* Note: a wildcard (asterisk-slash-asterisk) is included as a broad
* interoperability hint for generic clients and proxies. It is not a
* strict contract that every media type matching the wildcard will be
* accepted by canAcceptInput() (e.g., application/n-triples and
* application/rdf+xml are not accepted).
*/
export function getAcceptHeaders(connegEnabled, isContainer = false) {
const headers = {};

if (isContainer) {
headers['Accept-Post'] = connegEnabled
? `${RDF_TYPES.JSON_LD}, ${RDF_TYPES.TURTLE}, */*`
: `${RDF_TYPES.JSON_LD}, */*`;
? `${RDF_TYPES.JSON_LD}, application/json, ${RDF_TYPES.TURTLE}, ${RDF_TYPES.N3}, */*`
: `${RDF_TYPES.JSON_LD}, application/json, */*`;
}

headers['Accept-Put'] = connegEnabled
? `${RDF_TYPES.JSON_LD}, ${RDF_TYPES.TURTLE}, */*`
: `${RDF_TYPES.JSON_LD}, */*`;
? `${RDF_TYPES.JSON_LD}, application/json, ${RDF_TYPES.TURTLE}, ${RDF_TYPES.N3}, */*`
: `${RDF_TYPES.JSON_LD}, application/json, */*`;
Comment on lines 227 to +234

Copilot AI Apr 30, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getAcceptHeaders() still doesn’t advertise text/n3 even though canAcceptInput() accepts N3 when conneg is enabled (SUPPORTED_INPUT includes text/n3). This makes the advertised Accept-Post/Accept-Put contract incomplete for clients that rely on those headers. Consider adding text/n3 to the conneg-enabled Accept-Post and Accept-Put values (or, if N3 support is intentionally not advertised, add a short comment explaining why the header differs from the accepted set).

Copilot uses AI. Check for mistakes.
Comment on lines 226 to +234

Copilot AI Apr 30, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new application/json addition to Accept-Post/Accept-Put isn’t directly asserted in tests (the new ACL 415 tests set their own Accept-Put/Accept headers and would pass even if getAcceptHeaders() regressed). Consider adding/adjusting a test that verifies a normal OPTIONS/GET response advertises application/json in Accept-Put (and Accept-Post for containers) so the advertised contract stays aligned going forward.

Copilot uses AI. Check for mistakes.

Comment on lines 226 to 235

Copilot AI Apr 30, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getAcceptHeaders() now advertises application/json for container POST/Accept-Post, but the 415 error message on the POST path still states only application/ld+json is supported when conneg is disabled (see src/handlers/container.js 44-50). To keep the advertised contract and error messaging consistent, update the POST 415 response to list both application/ld+json and application/json (and ideally mirror the PUT handler by setting the relevant Accept-* headers on the 415 response).

Copilot uses AI. Check for mistakes.
headers['Accept-Patch'] = 'text/n3, application/sparql-update';

Expand Down
232 changes: 232 additions & 0 deletions test/conneg.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,34 @@ describe('Content Negotiation (conneg enabled)', () => {
assert.ok(acceptPost && acceptPost.includes('text/turtle'),
'Accept-Post should include text/turtle');
});

it('should advertise N3 support in Accept-Put when conneg enabled', async () => {
const res = await request('/connegtest/public/alice.json');
const acceptPut = res.headers.get('Accept-Put');
assert.ok(acceptPut && acceptPut.includes('text/n3'),
'Accept-Put should include text/n3 (canAcceptInput accepts it under conneg)');
});

it('should advertise N3 support in Accept-Post for containers when conneg enabled', async () => {
const res = await request('/connegtest/public/');
const acceptPost = res.headers.get('Accept-Post');
assert.ok(acceptPost && acceptPost.includes('text/n3'),
'Accept-Post should include text/n3 (canAcceptInput accepts it under conneg)');
});

it('should advertise application/json in Accept-Put', async () => {
const res = await request('/connegtest/public/alice.json');
const acceptPut = res.headers.get('Accept-Put');
assert.ok(acceptPut && acceptPut.includes('application/json'),
'Accept-Put should include application/json (canAcceptInput treats it as a JSON-LD alias)');
});

it('should advertise application/json in Accept-Post for containers', async () => {
const res = await request('/connegtest/public/');
const acceptPost = res.headers.get('Accept-Post');
assert.ok(acceptPost && acceptPost.includes('application/json'),
'Accept-Post should include application/json (canAcceptInput treats it as a JSON-LD alias)');
});
});

// Regression coverage for #294 — Solid convention dotfiles (.acl, .meta)
Expand Down Expand Up @@ -261,6 +289,107 @@ describe('Content Negotiation (conneg enabled)', () => {
'round-tripped JSON-LD should have at least one @-keyword');
});
});

// ACL resources require a JSON-LD payload (application/ld+json or
// application/json) on PUT regardless of conneg setting: round-trip
// serialization between JSON-LD and Turtle has known limitations
// that can cause data loss. See #295.
describe('ACL content-type guard (#295)', () => {
const aclJsonLd = {
'@context': { acl: 'http://www.w3.org/ns/auth/acl#' },
'@graph': [
{
'@id': '#owner',
'@type': 'acl:Authorization',
'acl:agent': { '@id': '#me' },
'acl:accessTo': { '@id': './' },
'acl:mode': [{ '@id': 'acl:Read' }, { '@id': 'acl:Write' }, { '@id': 'acl:Control' }]
}
]
};

it('rejects text/turtle PUT to .acl with 415', async () => {
const turtle = `
@prefix acl: <http://www.w3.org/ns/auth/acl#>.
<#owner> a acl:Authorization;
acl:mode acl:Read.
`;
const res = await request('/connegtest/public/turtle-reject.acl', {
method: 'PUT',
headers: { 'Content-Type': 'text/turtle' },
body: turtle,
auth: 'connegtest'
});
assertStatus(res, 415);
assertHeaderContains(res, 'Accept', 'application/ld+json');
assertHeaderContains(res, 'Accept', 'application/json');
assertHeaderContains(res, 'Accept-Put', 'application/ld+json');
assertHeaderContains(res, 'Accept-Put', 'application/json');
});
Comment on lines +323 to +328

Copilot AI Apr 30, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These 415 tests only assert that Accept / Accept-Put contain application/ld+json, but this PR also advertises application/json as supported for ACL PUT. Consider asserting application/json is present too, so the header contract described in the PR can’t regress silently.

Copilot uses AI. Check for mistakes.

it('rejects text/n3 PUT to .acl with 415', async () => {
const res = await request('/connegtest/public/n3-reject.acl', {
method: 'PUT',
headers: { 'Content-Type': 'text/n3' },
body: '@prefix acl: <http://www.w3.org/ns/auth/acl#>. <#x> a acl:Authorization.',
auth: 'connegtest'
});
assertStatus(res, 415);
assertHeaderContains(res, 'Accept', 'application/ld+json');
assertHeaderContains(res, 'Accept', 'application/json');
assertHeaderContains(res, 'Accept-Put', 'application/ld+json');
assertHeaderContains(res, 'Accept-Put', 'application/json');
});
Comment on lines +330 to +342

Copilot AI Apr 30, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only the first rejection test asserts Accept-Put; the other 415 cases in this block (e.g. text/n3) only assert Accept. Since the guard is intended to return both Accept and Accept-Put, it would be good to assert Accept-Put in each rejection test to prevent regressions where the header is accidentally omitted for some branches/content-types.

Copilot uses AI. Check for mistakes.

it('rejects text/plain PUT to .acl with 415 (URL-extension protection)', async () => {
const res = await request('/connegtest/public/plain-reject.acl', {
method: 'PUT',
headers: { 'Content-Type': 'text/plain' },
body: 'arbitrary text',
auth: 'connegtest'
});
assertStatus(res, 415);
assertHeaderContains(res, 'Accept', 'application/ld+json');
assertHeaderContains(res, 'Accept', 'application/json');
assertHeaderContains(res, 'Accept-Put', 'application/ld+json');
assertHeaderContains(res, 'Accept-Put', 'application/json');
});

it('rejects PUT to .acl with no Content-Type with 415', async () => {
// Use Uint8Array body so fetch() doesn't auto-set Content-Type
// (which it does for string bodies: text/plain;charset=UTF-8).
const res = await request('/connegtest/public/no-ct-reject.acl', {
method: 'PUT',
body: new Uint8Array([1, 2, 3, 4]),
auth: 'connegtest'
});
assertStatus(res, 415);
assertHeaderContains(res, 'Accept', 'application/ld+json');
assertHeaderContains(res, 'Accept', 'application/json');
assertHeaderContains(res, 'Accept-Put', 'application/ld+json');
assertHeaderContains(res, 'Accept-Put', 'application/json');
});
Comment on lines +358 to +371

Copilot AI Apr 30, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These “no Content-Type” tests likely still send a Content-Type header because request() uses fetch(), and when body is a JS string the Fetch implementation auto-sets Content-Type: text/plain;charset=UTF-8. That means the test isn’t actually validating the missing-Content-Type behavior. To truly omit Content-Type, send a Buffer/Uint8Array body (or explicitly set headers with no Content-Type and use a non-string body) so fetch won’t infer one.

Copilot uses AI. Check for mistakes.

it('accepts application/ld+json PUT to .acl', async () => {
const res = await request('/connegtest/public/jsonld-accept.acl', {
method: 'PUT',
headers: { 'Content-Type': 'application/ld+json' },
body: JSON.stringify(aclJsonLd),
auth: 'connegtest'
});
assert.ok(res.status < 300, `JSON-LD PUT to .acl should succeed, got ${res.status}`);
});

it('accepts application/json PUT to .acl', async () => {
const res = await request('/connegtest/public/json-accept.acl', {
method: 'PUT',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify(aclJsonLd),
auth: 'connegtest'
});
assert.ok(res.status < 300, `application/json PUT to .acl should succeed, got ${res.status}`);
});
});
Comment on lines +297 to +392

Copilot AI Apr 30, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests only exercise the .acl guard with conneg enabled, but the PR/issue states the restriction should apply regardless of the conneg setting (and the default mode is conneg disabled). Adding at least one test in the “conneg disabled - default” section for a Turtle/N3/plain PUT to .acl would prevent regressions in the primary deployment configuration.

Copilot uses AI. Check for mistakes.
});

describe('Content Negotiation (conneg disabled - default)', () => {
Expand Down Expand Up @@ -352,6 +481,109 @@ describe('Content Negotiation (conneg disabled - default)', () => {
assert.ok(!acceptPut || !acceptPut.includes('text/turtle'),
'Accept-Put should NOT include text/turtle when conneg disabled');
});

it('should advertise application/json in Accept-Put when conneg disabled', async () => {
const res = await request('/noconneg/public/');
const acceptPut = res.headers.get('Accept-Put');
assert.ok(acceptPut && acceptPut.includes('application/json'),
'Accept-Put should include application/json (canAcceptInput treats it as a JSON-LD alias)');
});

Copilot AI Apr 30, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The conneg-disabled header tests assert Accept-Put includes application/json, but there’s no assertion for the container Accept-Post header in the same mode even though getAcceptHeaders() now advertises application/json there too. Adding a conneg-disabled Accept-Post assertion would prevent regressions in container POST capability discovery.

Suggested change
it('should advertise application/json in Accept-Post when conneg disabled', async () => {
const res = await request('/noconneg/public/');
const acceptPost = res.headers.get('Accept-Post');
assert.ok(acceptPost && acceptPost.includes('application/json'),
'Accept-Post should include application/json (canAcceptInput treats it as a JSON-LD alias)');
});

Copilot uses AI. Check for mistakes.
it('should advertise application/json in Accept-Post when conneg disabled', async () => {
const res = await request('/noconneg/public/');
const acceptPost = res.headers.get('Accept-Post');
assert.ok(acceptPost && acceptPost.includes('application/json'),
'Accept-Post should include application/json (canAcceptInput treats it as a JSON-LD alias)');
});

it('should not advertise text/n3 in Accept-Put when conneg disabled', async () => {
const res = await request('/noconneg/public/');
const acceptPut = res.headers.get('Accept-Put');
assert.ok(!acceptPut || !acceptPut.includes('text/n3'),
'Accept-Put should NOT include text/n3 when conneg disabled');
});
});

// The .acl content-type guard applies regardless of conneg setting (#295).
// The default deployment configuration is conneg disabled, so ensure the
// guard fires there too.
describe('ACL content-type guard (#295) — conneg disabled', () => {
const aclJsonLd = {
'@context': { acl: 'http://www.w3.org/ns/auth/acl#' },
'@graph': [
{
'@id': '#owner',
'@type': 'acl:Authorization',
'acl:agent': { '@id': '#me' },
'acl:accessTo': { '@id': './' },
'acl:mode': [{ '@id': 'acl:Read' }, { '@id': 'acl:Write' }, { '@id': 'acl:Control' }]
}
]
};

it('rejects text/turtle PUT to .acl with 415', async () => {
const turtle = `@prefix acl: <http://www.w3.org/ns/auth/acl#>. <#x> a acl:Authorization.`;
Comment on lines +507 to +525

Copilot AI Apr 30, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR description says “add 5 test cases”, but this change adds more (a full ACL guard suite under conneg enabled plus additional coverage under conneg disabled, including missing Content-Type cases). Please update the PR description/test counts so reviewers and release notes match the actual scope.

Copilot uses AI. Check for mistakes.
const res = await request('/noconneg/public/turtle-reject.acl', {
method: 'PUT',
headers: { 'Content-Type': 'text/turtle' },
body: turtle,
auth: 'noconneg'
});
assertStatus(res, 415);
assertHeaderContains(res, 'Accept', 'application/ld+json');
assertHeaderContains(res, 'Accept', 'application/json');
assertHeaderContains(res, 'Accept-Put', 'application/ld+json');
assertHeaderContains(res, 'Accept-Put', 'application/json');
});
Comment on lines +524 to +537

Copilot AI Apr 30, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the conneg-disabled guard tests, the 415 assertions currently only check Accept. Since the implementation adds Accept-Put as well, consider asserting Accept-Put here too so the default deployment mode covers the full response contract.

Copilot uses AI. Check for mistakes.

it('rejects text/plain PUT to .acl with 415', async () => {
const res = await request('/noconneg/public/plain-reject.acl', {
method: 'PUT',
headers: { 'Content-Type': 'text/plain' },
body: 'arbitrary text',
auth: 'noconneg'
});
assertStatus(res, 415);
assertHeaderContains(res, 'Accept', 'application/ld+json');
assertHeaderContains(res, 'Accept', 'application/json');
assertHeaderContains(res, 'Accept-Put', 'application/ld+json');
assertHeaderContains(res, 'Accept-Put', 'application/json');
});

it('rejects PUT to .acl with no Content-Type with 415', async () => {
// Use Uint8Array body so fetch() doesn't auto-set Content-Type
// (which it does for string bodies: text/plain;charset=UTF-8).
const res = await request('/noconneg/public/no-ct-reject.acl', {
method: 'PUT',
body: new Uint8Array([1, 2, 3, 4]),
auth: 'noconneg'
});
assertStatus(res, 415);
assertHeaderContains(res, 'Accept', 'application/ld+json');
assertHeaderContains(res, 'Accept', 'application/json');
assertHeaderContains(res, 'Accept-Put', 'application/ld+json');
assertHeaderContains(res, 'Accept-Put', 'application/json');
});
Comment on lines +553 to +566

Copilot AI Apr 30, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same issue here: passing a string body to fetch() will typically cause it to infer Content-Type: text/plain;charset=UTF-8, so this test may not cover the truly-missing Content-Type case. Use a Buffer/Uint8Array body (and avoid setting Content-Type) to ensure the request is sent without the header.

Copilot uses AI. Check for mistakes.

it('accepts application/ld+json PUT to .acl', async () => {
const res = await request('/noconneg/public/jsonld-accept.acl', {
method: 'PUT',
headers: { 'Content-Type': 'application/ld+json' },
body: JSON.stringify(aclJsonLd),
auth: 'noconneg'
});
assert.ok(res.status < 300, `JSON-LD PUT to .acl should succeed, got ${res.status}`);
});

it('accepts application/json PUT to .acl', async () => {
const res = await request('/noconneg/public/json-accept.acl', {
method: 'PUT',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify(aclJsonLd),
auth: 'noconneg'
});
assert.ok(res.status < 300, `application/json PUT to .acl should succeed, got ${res.status}`);
});
Comment on lines +568 to +586

Copilot AI Apr 30, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The handler advertises application/json as an allowed ACL PUT media type; conneg-enabled tests cover this, but the conneg-disabled guard tests don’t. Adding an application/json acceptance test in this conneg-disabled block would ensure the advertised type is actually supported in the default mode.

Copilot uses AI. Check for mistakes.
Comment on lines +578 to +586

Copilot AI Apr 30, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This conneg-disabled ACL guard block includes an application/json acceptance test, which means the PR adds 5 conneg-disabled cases (and 11 total across enabled+disabled), not the 4/10 described in the PR description. Please align the PR description/test counts (or drop/adjust this test) so reviewers aren’t reconciling conflicting documentation.

Copilot uses AI. Check for mistakes.
});
});

Expand Down