Skip to content

fix(validate + session): the reserved skill:: namespace is writable, and only the opener may discard a session - #424

Merged
jrosskopf merged 2 commits into
mainfrom
fix/424-validate-reserved-skill-namespace
Aug 21, 2026
Merged

fix(validate + session): the reserved skill:: namespace is writable, and only the opener may discard a session#424
jrosskopf merged 2 commits into
mainfrom
fix/424-validate-reserved-skill-namespace

Conversation

@jrosskopf

@jrosskopf jrosskopf commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Two fixes, both surfaced by testing Heron against the real write path rather than through fixtures. They ship together because the second was found while verifying the first.


1. [[skill::<id>]] was resolvable but not writable (#424)

The documented way to reference a skill definition page — read.rs calls the namespace reserved, resolve constrains it on page_type = 'skill' (#212).

The validator did not know. It treated skill as a skill id, looked for a skill page called skill — which never exists — and refused every page using the form:

unknown_skill: wikilink references unknown skill `skill`

Two places were wrong, and fixing either alone changes the error rather than the outcome: the collection pass asked for the skill named skill instead of the link's id segment, and the existence check looked up the same wrong key.

How it surfaced: Heron's workshop formats reference a shared facilitation procedure exactly this way. Its Rust fixtures write straight to the store via FixtureBuilder, so validation never ran on them — the first thing to author a format the way a tenant would, an app test over the real write path, was refused immediately.

Worth noting for this repo too: FixtureBuilder bypassing validate means downstream suites can be green over content the gateway would reject.

Control: a reserved-namespace link to a skill that does not exist is still an error. Without it, the fix for a false positive becomes a dangling-link hole.


2. Anyone holding a session id could discard someone else's session (#425)

close_session {commit: true} re-checks the write ACL. {commit: false} checked nothing, deliberately — a caller whose grant was revoked mid-session must still be able to abandon their own work rather than wedge the page until the idle TTL.

That exemption could not tell the opener from anyone holding the session id:

bob (team-blue) → close_session {commit:false} on a team-red session → ok: true
the owner can then reopen the page — proving their session was destroyed

No content is read, so it is not a disclosure. It is a cross-engagement denial of service, and in a room the symptom is the facilitator's session dying mid-sentence.

Sessions now record the verified subject that opened them: admin passes, the opener passes, anyone else must be able to write the page — the same bar open_session sets. An unknown session id is treated as nothing to protect, so it cannot be distinguished from a forbidden one by probing.

Control: the opener can still discard their own session. A fix that simply required the write ACL would strand exactly the caller the exemption was written for.

Found by an AI review of Heron's P5 work and reproduced before fixing.


Verification

1356 tests pass, fmt and clippy clean.

🤖 Generated with Claude Code

… resolvable

`[[skill::<id>]]` is the documented way to reference a skill DEFINITION page:
`read.rs` calls the namespace reserved and `resolve` constrains it on
`page_type = 'skill'` (#212). The validator did not know. It treated `skill` as
a skill id, looked for a skill page called `skill` — which never exists — and
refused every page using the form with `unknown_skill`.

So a page that resolved correctly could not be WRITTEN. Two places were wrong,
and fixing one alone changes the error rather than the outcome: the collection
pass asked for the skill named `skill` instead of the link's id segment, and the
existence check looked up the same wrong key.

Found from Heron. Its workshop formats reference a shared facilitation
procedure exactly this way (its BR-WS-2), and its Rust fixtures write straight
to the store, so validation never ran on them. The first thing to author a
format the way a tenant actually would — an app test over the real write path —
was refused immediately.

The control matters as much as the fix: a reserved-namespace link to a skill
that does NOT exist is still an error. Without it, exempting `skill::` turns
every mistyped skill reference into a silently dangling link — the failure
`unknown_skill` exists to prevent, reintroduced through the fix for its false
positive.

1355 tests pass, fmt and clippy clean.
… discard

`close_session {commit: true}` re-checks the write ACL. `{commit: false}`
checked nothing, deliberately: a caller whose grant was revoked mid-session must
still be able to abandon their own work rather than wedge the page until the
idle TTL.

That exemption could not tell the OPENER from anyone holding the session id. A
caller on another engagement could terminate a live workshop in a room they
have no relationship to:

    bob (team-blue) -> close_session {commit: false} on a team-red session
      -> ok: true
    the owner can then reopen the page — proving their session was destroyed

No content is read, so it is not a disclosure. It is a cross-engagement denial
of service, and in a room the symptom is the facilitator's session dying
mid-sentence.

Sessions now record the verified subject that opened them, so the exemption
protects the right person: admin passes, the opener passes, anyone else must be
able to write the page — the same bar `open_session` sets. An unknown session id
is treated as nothing to protect, so it cannot be distinguished from a forbidden
one by probing.

The control is the half that keeps the original property: the opener can still
discard their own session. A fix that simply required the write ACL here would
strand exactly the caller the exemption was written for.

Found by an AI review of Heron's P5 work (2026-08-21) and reproduced before
fixing. Same review found three issues in Heron's own tests, all fixed there.

1356 tests pass, fmt and clippy clean.
@jrosskopf jrosskopf changed the title fix(validate): the reserved skill:: namespace is writable, not just resolvable fix(validate + session): the reserved skill:: namespace is writable, and only the opener may discard a session Aug 21, 2026
@jrosskopf
jrosskopf merged commit 02b9304 into main Aug 21, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant