fix(mcp): write_acl safety check + footgun docs - #498
Merged
melvincarvalho merged 1 commit intoMay 18, 2026
Merged
melvincarvalho merged 1 commit into
melvincarvalho merged 1 commit into
Conversation
Copy link
Copy Markdown
Contributor
Sorry, something went wrong.
Live-fire smoke of #497 surfaced two real gotchas with the new write_acl tool: 1. Relative WebID paths in 'agents' resolve against the .acl URL, not the pod root. ../profile/card.jsonld#me writes to a different absolute URI depending on where the .acl is — easy to lock yourself out. 2. There was no server-side check that the resulting ACL would keep the caller in control. This commit: - Adds a safety check in write_acl that parses the proposed ACL with its real URL (so relative agents resolve correctly), then checks whether any authorization grants Control to the caller. If not, refuses with an explanatory error pointing at the most common cause (relative WebIDs) and the workaround for legitimate ownership transfer. - Adds a 'Footguns' section to docs/mcp.md documenting the relative- vs-absolute WebID rule with concrete right/wrong examples, the new safety check, and the SSE keep-alive caveat. Tests: 27 passing (was 26). New test: write_acl refuses an ACL that grants Control only to a foreign WebID.
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Live-fire smoke of #497 surfaced two real gotchas with the new `write_acl` tool. Fixing both.
The bugs
1. write_acl could lock you out
Relative WebID paths in `agents` (e.g. `../profile/card.jsonld#me`) resolve against the `.acl` URL, not the pod root. Writing an ACL with a wrong-relative agent silently produced an ACL that didn't actually grant the caller Control — and on the next read_acl, they'd be locked out. AuthenticatedAgent Write was the only escape hatch.
2. No clear docs on URL resolution
The `agents` field accepting URIs (and the URL resolution rules) was implicit. Users had to know WAC well enough to predict resolution behavior.
The fixes
Legitimate ownership transfer
Still possible — just two steps:
Test plan